Skip to content

Commit

Permalink
Fix lints and run cargo clippy in CI
Browse files Browse the repository at this point in the history
Signed-off-by: Francisco Javier Honduvilla Coto <[email protected]>
  • Loading branch information
javierhonduco committed Mar 17, 2024
1 parent 69f89d1 commit 5faf6f2
Show file tree
Hide file tree
Showing 7 changed files with 41 additions and 27 deletions.
8 changes: 6 additions & 2 deletions .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ on:
push:

jobs:
tests:
ci:
runs-on: ubuntu-22.04
permissions:
id-token: write
Expand All @@ -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
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
3 changes: 1 addition & 2 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
/target
flame.svg
src/bpf/bpf.rs
src/bpf/profiler.rs
src/bpf/*_skel.rs
5 changes: 2 additions & 3 deletions build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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!");
Expand Down
4 changes: 2 additions & 2 deletions src/bpf/mod.rs
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
pub mod bindings;
pub mod bpf;
pub mod profiler_bindings;
pub mod profiler_skel;
3 changes: 2 additions & 1 deletion src/bpf/bindings.rs → src/bpf/profiler_bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,15 @@

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 {}
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 {
Expand Down
1 change: 1 addition & 0 deletions src/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use std::path::PathBuf;
use data_encoding::HEXUPPER;
use memmap2;

#[allow(dead_code)]
enum BuildId {
Gnu(String),
Go(String),
Expand Down
44 changes: 27 additions & 17 deletions src/profiler.rs
Original file line number Diff line number Diff line change
@@ -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::{
Expand All @@ -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,
Expand All @@ -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;

Expand Down Expand Up @@ -205,11 +208,12 @@ enum MappingType {
}

#[derive(Clone)]
struct ProcessInfo {
pub struct ProcessInfo {
mappings: Vec<ExecutableMapping>,
}

struct ObjectFileInfo {
#[allow(dead_code)]
pub struct ObjectFileInfo {
file: fs::File,
path: PathBuf,
// p_offset, p_vaddr
Expand All @@ -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<String>,
Expand Down Expand Up @@ -259,7 +264,9 @@ fn in_memory_unwind_info(path: &str) -> anyhow::Result<Vec<stack_unwind_row_t>>
};
unwind_info.push(row)
}
None => {}
None => {
// todo: cleanup
}
}
last_function_end_addr = Some(*end_addr);
}
Expand Down Expand Up @@ -371,7 +378,7 @@ impl Collector {
}

pub fn finish(&self) -> Vec<SymbolizedAggregatedProfile> {
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();
Expand All @@ -388,14 +395,16 @@ 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<native_stack_t>,
kstack: Option<native_stack_t>,
count: u64,
}

#[derive(Default, Debug)]
#[allow(dead_code)]
pub struct SymbolizedAggregatedSample {
pid: i32,
pub ustack: Vec<String>,
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -650,7 +659,7 @@ impl Profiler<'_> {
}

fn persist_unwind_info(&self, live_shard: &Vec<stack_unwind_row_t>) {
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 {
Expand Down Expand Up @@ -681,15 +690,14 @@ 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")
.get(&pid)
.unwrap()
.mappings
.iter()
.enumerate()
{
if self.native_unwind_state.shard_index > MAX_UNWIND_INFO_SHARDS {
error!("No more unwind info shards available");
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -1082,7 +1090,9 @@ impl Profiler<'_> {
load_address = thing.address.0;
}
}
_ => {}
_ => {
// todo: cleanup
}
}

mappings.push(ExecutableMapping {
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit 5faf6f2

Please sign in to comment.