From 5faf6f2e7df5734c5b595f457f7888565482ce1d Mon Sep 17 00:00:00 2001 From: Francisco Javier Honduvilla Coto Date: Sat, 16 Mar 2024 18:03:04 +0000 Subject: [PATCH] Fix lints and run `cargo clippy` in CI Signed-off-by: Francisco Javier Honduvilla Coto --- .github/workflows/build.yml | 8 +++- .gitignore | 3 +- build.rs | 5 +-- src/bpf/mod.rs | 4 +- src/bpf/{bindings.rs => profiler_bindings.rs} | 3 +- src/object.rs | 1 + src/profiler.rs | 44 ++++++++++++------- 7 files changed, 41 insertions(+), 27 deletions(-) rename src/bpf/{bindings.rs => profiler_bindings.rs} (84%) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 420d5f6..41fe39c 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -4,7 +4,7 @@ on: push: jobs: - tests: + ci: runs-on: ubuntu-22.04 permissions: id-token: write @@ -14,4 +14,8 @@ jobs: - uses: DeterminateSystems/nix-installer-action@main - uses: DeterminateSystems/magic-nix-cache-action@main - name: Run `cargo check` - run: nix develop --command cargo check \ No newline at end of file + run: nix develop --command cargo check + - name: Run `cargo clippy` + run: nix develop --command cargo clippy + - name: Run `cargo fmt` + run: nix develop --command cargo fmt \ No newline at end of file diff --git a/.gitignore b/.gitignore index 9c5e1a7..810a477 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,3 @@ /target flame.svg -src/bpf/bpf.rs -src/bpf/profiler.rs +src/bpf/*_skel.rs \ No newline at end of file diff --git a/build.rs b/build.rs index dec3a46..a43402d 100644 --- a/build.rs +++ b/build.rs @@ -8,7 +8,7 @@ use std::path::Path; const PROFILER_BPF_HEADER: &str = "./src/bpf/profiler.h"; const PROFILER_BPF_SOURCE: &str = "./src/bpf/profiler.bpf.c"; -const PROFILER_SKELETON: &str = "./src/bpf/bpf.rs"; +const PROFILER_SKELETON: &str = "./src/bpf/profiler_skel.rs"; fn main() { // This is necessary but not sure why, this should be passed elsewhere @@ -24,9 +24,8 @@ fn main() { .generate() .expect("Unable to generate bindings"); - // Write the bindings to the $OUT_DIR/bindings.rs file. let out_path = PathBuf::from(env::var("OUT_DIR").unwrap()); - let bindings_out_file = out_path.join("bindings.rs"); + let bindings_out_file = out_path.join("profiler_bindings.rs"); bindings .write_to_file(bindings_out_file) .expect("Couldn't write bindings!"); diff --git a/src/bpf/mod.rs b/src/bpf/mod.rs index a5e2cd3..d82ba9a 100644 --- a/src/bpf/mod.rs +++ b/src/bpf/mod.rs @@ -1,2 +1,2 @@ -pub mod bindings; -pub mod bpf; +pub mod profiler_bindings; +pub mod profiler_skel; diff --git a/src/bpf/bindings.rs b/src/bpf/profiler_bindings.rs similarity index 84% rename from src/bpf/bindings.rs rename to src/bpf/profiler_bindings.rs index d2aa15d..480119e 100644 --- a/src/bpf/bindings.rs +++ b/src/bpf/profiler_bindings.rs @@ -4,7 +4,7 @@ use plain::Plain; -include!(concat!(env!("OUT_DIR"), "/bindings.rs")); +include!(concat!(env!("OUT_DIR"), "/profiler_bindings.rs")); unsafe impl Plain for stack_count_key_t {} unsafe impl Plain for native_stack_t {} @@ -12,6 +12,7 @@ unsafe impl Plain for Event {} unsafe impl Plain for process_info_t {} unsafe impl Plain for unwind_info_chunks_t {} +#[allow(clippy::derivable_impls)] impl Default for stack_count_key_t { fn default() -> Self { Self { diff --git a/src/object.rs b/src/object.rs index c40251d..767770e 100644 --- a/src/object.rs +++ b/src/object.rs @@ -15,6 +15,7 @@ use std::path::PathBuf; use data_encoding::HEXUPPER; use memmap2; +#[allow(dead_code)] enum BuildId { Gnu(String), Go(String), diff --git a/src/profiler.rs b/src/profiler.rs index 362bee7..5b5690f 100644 --- a/src/profiler.rs +++ b/src/profiler.rs @@ -1,6 +1,6 @@ -use tracing::{debug, error, info, span, Level}; +use tracing::{debug, error, span, Level}; -use crate::bpf::bpf::{ProfilerSkel, ProfilerSkelBuilder}; +use crate::bpf::profiler_skel::{ProfilerSkel, ProfilerSkelBuilder}; use crate::object::{build_id, elf_load, is_dynamic, is_go}; use crate::perf_events::setup_perf_event; use crate::unwind_info::{ @@ -25,7 +25,7 @@ use std::thread; use std::time::Duration; use std::time::Instant; -use crate::bpf::bindings::*; +use crate::bpf::profiler_bindings::*; fn symbolize_profile( profile: &RawAggregatedProfile, @@ -38,8 +38,11 @@ fn symbolize_profile( let _ = fetch_symbols_for_profile(&mut addresses_per_sample, profile, procs, objs); // best effort for sample in profile { - let mut symbolized_sample: SymbolizedAggregatedSample = - SymbolizedAggregatedSample::default(); + let mut symbolized_sample: SymbolizedAggregatedSample = SymbolizedAggregatedSample { + pid: sample.pid, + count: sample.count, + ..Default::default() + }; symbolized_sample.pid = sample.pid; symbolized_sample.count = sample.count; @@ -205,11 +208,12 @@ enum MappingType { } #[derive(Clone)] -struct ProcessInfo { +pub struct ProcessInfo { mappings: Vec, } -struct ObjectFileInfo { +#[allow(dead_code)] +pub struct ObjectFileInfo { file: fs::File, path: PathBuf, // p_offset, p_vaddr @@ -226,6 +230,7 @@ enum Unwinder { } #[derive(Debug, Clone)] +#[allow(dead_code)] struct ExecutableMapping { // No build id means either JIT or that we could not fetch it. Change this. build_id: Option, @@ -259,7 +264,9 @@ fn in_memory_unwind_info(path: &str) -> anyhow::Result> }; unwind_info.push(row) } - None => {} + None => { + // todo: cleanup + } } last_function_end_addr = Some(*end_addr); } @@ -371,7 +378,7 @@ impl Collector { } pub fn finish(&self) -> Vec { - let span: span::EnteredSpan = span!(Level::DEBUG, "symbolize_profiles").entered(); + let _span: span::EnteredSpan = span!(Level::DEBUG, "symbolize_profiles").entered(); debug!("Collector::finish {}", self.profiles.len()); let mut r = Vec::new(); @@ -388,7 +395,8 @@ const MAX_UNWIND_INFO_SHARDS: u64 = 50; const SHARD_CAPACITY: usize = MAX_UNWIND_TABLE_SIZE as usize; const PERF_BUFFER_PAGES: usize = 512 * 1024; -struct RawAggregatedSample { +#[allow(dead_code)] +pub struct RawAggregatedSample { pid: i32, ustack: Option, kstack: Option, @@ -396,6 +404,7 @@ struct RawAggregatedSample { } #[derive(Default, Debug)] +#[allow(dead_code)] pub struct SymbolizedAggregatedSample { pid: i32, pub ustack: Vec, @@ -514,7 +523,7 @@ impl Profiler<'_> { } }); - let mut start_time: Instant = Instant::now(); + let start_time: Instant = Instant::now(); let mut time_since_last_scheduled_collection: Instant = Instant::now(); loop { @@ -650,7 +659,7 @@ impl Profiler<'_> { } fn persist_unwind_info(&self, live_shard: &Vec) { - let span = span!(Level::DEBUG, "persist_unwind_info").entered(); + let _span = span!(Level::DEBUG, "persist_unwind_info").entered(); let key = self.native_unwind_state.shard_index.to_ne_bytes(); let val = unsafe { @@ -681,7 +690,7 @@ impl Profiler<'_> { let mut got_some_unwind_info: bool = true; // Get unwind info - for (_i, mapping) in self + for mapping in self .procs .lock() .expect("lock") @@ -689,7 +698,6 @@ impl Profiler<'_> { .unwrap() .mappings .iter() - .enumerate() { if self.native_unwind_state.shard_index > MAX_UNWIND_INFO_SHARDS { error!("No more unwind info shards available"); @@ -836,7 +844,7 @@ impl Profiler<'_> { let last_pc = found_unwind_info[found_unwind_info.len() - 1].pc; debug!("~~ PC range {:x}-{:x}", first_pc, last_pc,); - let mut current_chunk = &found_unwind_info[..]; + let mut current_chunk; let mut rest_chunk = &found_unwind_info[..]; loop { @@ -1082,7 +1090,9 @@ impl Profiler<'_> { load_address = thing.address.0; } } - _ => {} + _ => { + // todo: cleanup + } } mappings.push(ExecutableMapping { @@ -1138,7 +1148,7 @@ impl Profiler<'_> { } } - mappings.sort_by(|a, _b| a.start_addr.cmp(&a.start_addr)); + mappings.sort_by_key(|k| k.start_addr.cmp(&k.start_addr)); let proc_info = ProcessInfo { mappings }; self.procs .clone()