Skip to content

Commit

Permalink
Improve unwind info code organisation
Browse files Browse the repository at this point in the history
Split it in different files, added some more docs, and call the unwind
info optimisers directly.

Error handling improvements, more docs, and especially more tests coming
later. As this is a rather load-bearing part of the project, any issues
pop up pretty quickly in CI.

Test Plan
=========

CI
  • Loading branch information
javierhonduco committed Oct 31, 2024
1 parent b72407d commit 3118a32
Show file tree
Hide file tree
Showing 9 changed files with 641 additions and 634 deletions.
2 changes: 1 addition & 1 deletion src/bpf/profiler_bindings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
use plain::Plain;
use std::ops::Add;

use crate::unwind_info::CompactUnwindRow;
use crate::unwind_info::types::CompactUnwindRow;

include!(concat!(env!("OUT_DIR"), "/profiler_bindings.rs"));

Expand Down
15 changes: 5 additions & 10 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,8 @@ use lightswitch_metadata_provider::metadata_provider::ThreadSafeGlobalMetadataPr
use lightswitch::profile::symbolize_profile;
use lightswitch::profile::{fold_profile, to_pprof};
use lightswitch::profiler::{Profiler, ProfilerConfig};
use lightswitch::unwind_info::in_memory_unwind_info;
use lightswitch::unwind_info::remove_redundant;
use lightswitch::unwind_info::remove_unnecesary_markers;
use lightswitch::unwind_info::UnwindInfoBuilder;
use lightswitch::unwind_info::compact_unwind_info;
use lightswitch::unwind_info::CompactUnwindInfoBuilder;
use lightswitch_object::ObjectFile;

const SAMPLE_FREQ_RANGE: RangeInclusive<usize> = 1..=1009;
Expand Down Expand Up @@ -237,10 +235,7 @@ fn main() -> Result<(), Box<dyn Error>> {
let args = Cli::parse();

if let Some(path) = args.show_unwind_info {
let mut unwind_info = in_memory_unwind_info(&path).unwrap();
remove_unnecesary_markers(&mut unwind_info);
remove_redundant(&mut unwind_info);

let unwind_info = compact_unwind_info(&path).unwrap();
for compact_row in unwind_info {
let pc = compact_row.pc;
let cfa_type = compact_row.cfa_type;
Expand Down Expand Up @@ -272,8 +267,8 @@ fn main() -> Result<(), Box<dyn Error>> {
if let Some(path) = args.show_info {
let objet_file = ObjectFile::new(&PathBuf::from(path.clone())).unwrap();
println!("build id {:?}", objet_file.build_id());
let unwind_info: Result<UnwindInfoBuilder<'_>, anyhow::Error> =
UnwindInfoBuilder::with_callback(&path, |_| {});
let unwind_info: Result<CompactUnwindInfoBuilder<'_>, anyhow::Error> =
CompactUnwindInfoBuilder::with_callback(&path, |_| {});
println!("unwind info {:?}", unwind_info.unwrap().process());

return Ok(());
Expand Down
60 changes: 26 additions & 34 deletions src/profiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,9 @@ use crate::bpf::tracers_bindings::*;
use crate::bpf::tracers_skel::{TracersSkel, TracersSkelBuilder};
use crate::collector::*;
use crate::perf_events::setup_perf_event;
use crate::unwind_info::compact_unwind_info;
use crate::unwind_info::log_unwind_info_sections;
use crate::unwind_info::CompactUnwindRow;
use crate::unwind_info::{in_memory_unwind_info, remove_redundant, remove_unnecesary_markers};
use crate::unwind_info::types::CompactUnwindRow;
use crate::util::{get_online_cpus, summarize_address_range};
use lightswitch_object::ElfLoad;
use lightswitch_object::{BuildId, ExecutableId, ObjectFile};
Expand Down Expand Up @@ -1140,7 +1140,7 @@ impl Profiler<'_> {
executable_id: u64,
bucket_id: u32,
) {
let pages = crate::unwind_info::to_pages(unwind_info);
let pages = crate::unwind_info::pages::to_pages(unwind_info);
for page in pages {
let page_key = page_key_t {
file_offset: page.address,
Expand Down Expand Up @@ -1445,40 +1445,32 @@ impl Profiler<'_> {
)
.entered();

let mut found_unwind_info: Vec<CompactUnwindRow>;

match in_memory_unwind_info(&executable_path.to_string_lossy()) {
Ok(unwind_info) => {
found_unwind_info = unwind_info;
}
Err(e) => {
let executable_path_str = executable.path.to_string_lossy();
let known_naughty = executable_path_str.contains("libicudata");

// tracing doesn't support a level chosen at runtime: https://github.com/tokio-rs/tracing/issues/2730
if known_naughty {
debug!(
"failed to get unwind information for {} with {}",
executable_path_str, e
);
} else {
info!(
"failed to get unwind information for {} with {}",
executable_path_str, e
);
let found_unwind_info: Vec<CompactUnwindRow> =
match compact_unwind_info(&executable_path.to_string_lossy()) {
Ok(unwind_info) => unwind_info,
Err(e) => {
let executable_path_str = executable.path.to_string_lossy();
let known_naughty = executable_path_str.contains("libicudata");

// tracing doesn't support a level chosen at runtime: https://github.com/tokio-rs/tracing/issues/2730
if known_naughty {
debug!(
"failed to get unwind information for {} with {}",
executable_path_str, e
);
} else {
info!(
"failed to get unwind information for {} with {}",
executable_path_str, e
);

if let Err(e) = log_unwind_info_sections(&executable_path) {
warn!("log_unwind_info_sections failed with {}", e);
if let Err(e) = log_unwind_info_sections(&executable_path) {
warn!("log_unwind_info_sections failed with {}", e);
}
}
continue;
}
continue;
}
}
span.exit();

let span: span::EnteredSpan = span!(Level::DEBUG, "optimize unwind info").entered();
remove_unnecesary_markers(&mut found_unwind_info);
remove_redundant(&mut found_unwind_info);
};
span.exit();

// Evicting object files can get complicated real quick... this can be implemented once
Expand Down
Loading

0 comments on commit 3118a32

Please sign in to comment.