Skip to content

Commit

Permalink
Implement --unstable-presymbolicate
Browse files Browse the repository at this point in the history
  • Loading branch information
vvuk committed May 15, 2024
1 parent 5e41fd1 commit 81afd7e
Show file tree
Hide file tree
Showing 19 changed files with 580 additions and 16 deletions.
5 changes: 4 additions & 1 deletion fxprof-processed-profile/src/frame_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ impl FrameTable {
resource_table: &mut ResourceTable,
func_table: &mut FuncTable,
native_symbol_table: &mut NativeSymbols,
global_libs: &GlobalLibTable,
global_libs: &mut GlobalLibTable,
frame: InternalFrame,
) -> usize {
let addresses = &mut self.addresses;
Expand Down Expand Up @@ -73,6 +73,9 @@ impl FrameTable {
(Some(native_symbol), name_string_index)
}
None => {
// This isn't in the pre-provided symbol table, and we know it's in a library.
global_libs.add_lib_used_rva(lib_index, address);

let location_string = format!("0x{address:x}");
(None, string_table.index_for_string(&location_string))
}
Expand Down
45 changes: 44 additions & 1 deletion fxprof-processed-profile/src/global_lib_table.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use std::collections::BTreeSet;
use std::sync::Arc;

use serde::ser::{Serialize, Serializer};
Expand All @@ -14,6 +15,11 @@ pub struct GlobalLibTable {
used_libs: Vec<LibraryHandle>, // append-only for stable GlobalLibIndexes
lib_map: FastHashMap<LibraryInfo, LibraryHandle>,
used_lib_map: FastHashMap<LibraryHandle, GlobalLibIndex>,
/// We keep track of RVA addresses that exist in frames that are assigned to this
/// library, so that we can potentially provide symbolication info ahead of time.
/// This is here instead of in `LibraryInfo` because we don't want to serialize it,
/// and because it's currently a hack.
all_libs_seen_rvas: Vec<Option<BTreeSet<u32>>>,
}

impl GlobalLibTable {
Expand All @@ -23,6 +29,7 @@ impl GlobalLibTable {
used_libs: Vec::new(),
lib_map: FastHashMap::default(),
used_lib_map: FastHashMap::default(),
all_libs_seen_rvas: Vec::new(),
}
}

Expand All @@ -31,6 +38,7 @@ impl GlobalLibTable {
*self.lib_map.entry(lib.clone()).or_insert_with(|| {
let handle = LibraryHandle(all_libs.len());
all_libs.push(lib);
self.all_libs_seen_rvas.push(None);
handle
})
}
Expand All @@ -52,6 +60,19 @@ impl GlobalLibTable {
let handle = self.used_libs.get(index.0)?;
self.all_libs.get(handle.0)
}

pub fn add_lib_used_rva(&mut self, index: GlobalLibIndex, address: u32) {
let handle = self.used_libs.get(index.0).unwrap();
let lib_seen_rvas = self.all_libs_seen_rvas[handle.0].get_or_insert_with(BTreeSet::new);
lib_seen_rvas.insert(address);
}

pub fn lib_used_rva_iter(&self) -> UsedLibraryAddressesIterator {
UsedLibraryAddressesIterator {
next_used_lib_index: 0,
global_lib_table: self,
}
}
}

impl Serialize for GlobalLibTable {
Expand All @@ -73,4 +94,26 @@ impl Serialize for GlobalLibIndex {

/// The handle for a library, obtained from [`Profile::add_lib`](crate::Profile::add_lib).
#[derive(Debug, Clone, Copy, PartialOrd, Ord, PartialEq, Eq, Hash)]
pub struct LibraryHandle(usize);
pub struct LibraryHandle(pub(crate) usize);

pub struct UsedLibraryAddressesIterator<'a> {
next_used_lib_index: usize,
global_lib_table: &'a GlobalLibTable,
}

impl<'a> Iterator for UsedLibraryAddressesIterator<'a> {
type Item = (&'a LibraryInfo, Option<&'a BTreeSet<u32>>);

fn next(&mut self) -> Option<Self::Item> {
self.global_lib_table
.used_libs
.get(self.next_used_lib_index)
.map(|lib| {
self.next_used_lib_index += 1;
(
&self.global_lib_table.all_libs[lib.0],
self.global_lib_table.all_libs_seen_rvas[lib.0].as_ref(),
)
})
}
}
2 changes: 1 addition & 1 deletion fxprof-processed-profile/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ pub use category_color::CategoryColor;
pub use counters::CounterHandle;
pub use cpu_delta::CpuDelta;
pub use frame::{Frame, FrameFlags, FrameInfo};
pub use global_lib_table::LibraryHandle;
pub use global_lib_table::{LibraryHandle, UsedLibraryAddressesIterator};
pub use lib_mappings::LibMappings;
pub use library_info::{LibraryInfo, Symbol, SymbolTable};
pub use markers::*;
Expand Down
8 changes: 6 additions & 2 deletions fxprof-processed-profile/src/profile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::cpu_delta::CpuDelta;
use crate::fast_hash_map::FastHashMap;
use crate::frame::{Frame, FrameInfo};
use crate::frame_table::{InternalFrame, InternalFrameLocation};
use crate::global_lib_table::{GlobalLibTable, LibraryHandle};
use crate::global_lib_table::{GlobalLibTable, LibraryHandle, UsedLibraryAddressesIterator};
use crate::lib_mappings::LibMappings;
use crate::library_info::LibraryInfo;
use crate::process::{Process, ThreadHandle};
Expand Down Expand Up @@ -601,7 +601,7 @@ impl Profile {
flags: frame_info.flags,
category_pair: frame_info.category_pair,
};
let frame_index = thread.frame_index_for_frame(internal_frame, &self.global_libs);
let frame_index = thread.frame_index_for_frame(internal_frame, &mut self.global_libs);
prefix =
Some(thread.stack_index_for_stack(prefix, frame_index, frame_info.category_pair));
}
Expand Down Expand Up @@ -665,6 +665,10 @@ impl Profile {
fn contains_js_function(&self) -> bool {
self.threads.iter().any(|t| t.contains_js_function())
}

pub fn lib_used_rva_iter(&self) -> UsedLibraryAddressesIterator {
self.global_libs.lib_used_rva_iter()
}
}

impl Serialize for Profile {
Expand Down
2 changes: 1 addition & 1 deletion fxprof-processed-profile/src/thread.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ impl Thread {
pub fn frame_index_for_frame(
&mut self,
frame: InternalFrame,
global_libs: &GlobalLibTable,
global_libs: &mut GlobalLibTable,
) -> usize {
self.frame_table.index_for_frame(
&mut self.string_table,
Expand Down
2 changes: 2 additions & 0 deletions fxprof-processed-profile/tests/integration_tests/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,7 @@ fn profile_without_js() {
name: "libc_symbol_2".to_string(),
},
]))),
seen_rvas: None,
});
profile.add_lib_mapping(
process,
Expand All @@ -168,6 +169,7 @@ fn profile_without_js() {
debug_id: DebugId::from_breakpad("5C0A0D51EA1980DF43F203B4525BE9BE0").unwrap(),
arch: None,
symbol_table: None,
seen_rvas: None,
});
profile.add_lib_mapping(
process,
Expand Down
10 changes: 9 additions & 1 deletion samply-symbols/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -256,7 +256,7 @@ pub use crate::shared::{
MultiArchDisambiguator, OptionallySendFuture, PeCodeId, SourceFilePath, SymbolInfo,
SyncAddressInfo,
};
pub use crate::symbol_map::SymbolMap;
pub use crate::symbol_map::{SymbolMap, SymbolMapTrait};

pub struct SymbolManager<H: FileAndPathHelper> {
helper: Arc<H>,
Expand Down Expand Up @@ -304,6 +304,14 @@ where
/// Obtain a symbol map for the library, given the (partial) `LibraryInfo`.
/// At least the debug_id has to be given.
pub async fn load_symbol_map(&self, library_info: &LibraryInfo) -> Result<SymbolMap<H>, Error> {
if let Some((fl, symbol_map)) = self
.helper()
.as_ref()
.get_symbol_map_for_library(library_info)
{
return Ok(SymbolMap::with_symbol_map_trait(fl, symbol_map));
}

let debug_id = match library_info.debug_id {
Some(debug_id) => debug_id,
None => return Err(Error::NotEnoughInformationToIdentifySymbolMap),
Expand Down
10 changes: 10 additions & 0 deletions samply-symbols/src/shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use std::future::Future;
use std::marker::PhantomData;
use std::ops::{Deref, Range};
use std::str::FromStr;
use std::sync::Arc;

#[cfg(feature = "partial_read_stats")]
use bitvec::{bitvec, prelude::BitVec};
Expand All @@ -14,6 +15,7 @@ use object::FileFlags;
use uuid::Uuid;

use crate::mapped_path::MappedPath;
use crate::symbol_map::SymbolMapTrait;

pub type FileAndPathHelperError = Box<dyn std::error::Error + Send + Sync + 'static>;
pub type FileAndPathHelperResult<T> = std::result::Result<T, FileAndPathHelperError>;
Expand Down Expand Up @@ -402,6 +404,14 @@ pub trait FileAndPathHelper {
&self,
location: Self::FL,
) -> std::pin::Pin<Box<dyn OptionallySendFuture<Output = FileAndPathHelperResult<Self::F>> + '_>>;

/// Ask the helper to return a SymbolMap if it happens to have one available already.
fn get_symbol_map_for_library(
&self,
_info: &LibraryInfo,
) -> Option<(Self::FL, Arc<dyn SymbolMapTrait + Send + Sync>)> {
None
}
}

/// Provides synchronous access to the raw bytes of a file.
Expand Down
17 changes: 15 additions & 2 deletions samply-symbols/src/symbol_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ pub trait GetInnerSymbolMapWithLookupFramesExt<FC> {
enum InnerSymbolMap<FC> {
WithoutAddFile(Box<dyn GetInnerSymbolMap + Send + Sync>),
WithAddFile(Box<dyn GetInnerSymbolMapWithLookupFramesExt<FC> + Send + Sync>),
Direct(Arc<dyn SymbolMapTrait + Send + Sync>),
}

pub struct SymbolMap<H: FileAndPathHelper> {
Expand Down Expand Up @@ -74,10 +75,22 @@ impl<H: FileAndPathHelper> SymbolMap<H> {
}
}

pub fn with_symbol_map_trait(
debug_file_location: H::FL,
inner: Arc<dyn SymbolMapTrait + Send + Sync>,
) -> Self {
Self {
debug_file_location,
inner: InnerSymbolMap::Direct(inner),
helper: None,
}
}

fn inner(&self) -> &dyn SymbolMapTrait {
match &self.inner {
InnerSymbolMap::WithoutAddFile(inner) => inner.get_inner_symbol_map(),
InnerSymbolMap::WithAddFile(inner) => inner.get_inner_symbol_map().get_as_symbol_map(),
InnerSymbolMap::Direct(inner) => inner.as_ref(),
}
}

Expand Down Expand Up @@ -111,7 +124,7 @@ impl<H: FileAndPathHelper> SymbolMap<H> {
frames: Some(frames),
});
}
(None, _) | (_, InnerSymbolMap::WithoutAddFile(_)) => {
(None, _) | (_, InnerSymbolMap::WithoutAddFile(_)) | (_, InnerSymbolMap::Direct(_)) => {
return Some(AddressInfo {
symbol,
frames: None,
Expand Down Expand Up @@ -168,7 +181,7 @@ impl<H: FileAndPathHelper> SymbolMap<H> {
) -> Option<Vec<FrameDebugInfo>> {
let helper = self.helper.as_deref()?;
let inner = match &self.inner {
InnerSymbolMap::WithoutAddFile(_) => return None,
InnerSymbolMap::WithoutAddFile(_) | InnerSymbolMap::Direct(_) => return None,
InnerSymbolMap::WithAddFile(inner) => inner.get_inner_symbol_map(),
};

Expand Down
16 changes: 13 additions & 3 deletions samply/src/linux/profiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -534,6 +534,7 @@ fn run_profiler(
more_processes_request_receiver: Receiver<SamplerRequest>,
more_processes_reply_sender: Sender<bool>,
mut stop_receiver: oneshot::Receiver<()>,
unstable_presymbolicate: bool,
) {
// eprintln!("Running...");

Expand Down Expand Up @@ -659,9 +660,18 @@ fn run_profiler(

let profile = converter.finish();

let output_file = File::create(output_filename).unwrap();
let writer = BufWriter::new(output_file);
serde_json::to_writer(writer, &profile).expect("Couldn't write JSON");
{
let output_file = File::create(output_filename).unwrap();
let writer = BufWriter::new(output_file);
serde_json::to_writer(writer, &profile).expect("Couldn't write JSON");
}

if unstable_presymbolicate {
crate::shared::symbol_precog::presymbolicate(
&profile,
&output_filename.with_extension("syms.json"),
);
}
}

pub fn read_string_lossy<P: AsRef<Path>>(path: P) -> std::io::Result<String> {
Expand Down
9 changes: 9 additions & 0 deletions samply/src/mac/profiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,8 @@ pub fn start_recording(
..profile_creation_props
};

let unstable_presymbolicate = profile_creation_props.unstable_presymbolicate;

let (task_sender, task_receiver) = unbounded();

let sampler_thread = thread::spawn(move || {
Expand Down Expand Up @@ -210,6 +212,13 @@ pub fn start_recording(
to_writer(writer, &profile).expect("Couldn't write JSON");
}

if unstable_presymbolicate {
crate::shared::symbol_precog::presymbolicate(
&profile,
&output_file.with_extension("syms.json"),
);
}

if let Some(server_props) = server_props {
let libinfo_map = crate::profile_json_preparse::parse_libinfo_map_from_profile_file(
File::open(&output_file).expect("Couldn't open file we just wrote"),
Expand Down
10 changes: 10 additions & 0 deletions samply/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,14 @@ pub struct ProfileCreationArgs {
/// Create a separate thread for each CPU. Not supported on macOS
#[arg(long)]
per_cpu_threads: bool,

/// Emit .syms.json sidecar file containing gathered symbol info for all frames referenced by
/// this profile. With this file along with the profile, samply can load the profile
/// and provide symbols to the front end without needing debug files to be
/// available. (Unstable: will probably change to include the full information
/// in the profile.json, instead of a sidecar file.)
#[arg(long)]
unstable_presymbolicate: bool,
}

#[derive(Debug, Args)]
Expand Down Expand Up @@ -369,6 +377,7 @@ impl ImportArgs {
unlink_aux_files: self.profile_creation_args.unlink_aux_files,
create_per_cpu_threads: self.profile_creation_args.per_cpu_threads,
override_arch: self.override_arch.clone(),
unstable_presymbolicate: self.profile_creation_args.unstable_presymbolicate,
}
}

Expand Down Expand Up @@ -472,6 +481,7 @@ impl RecordArgs {
unlink_aux_files: self.profile_creation_args.unlink_aux_files,
create_per_cpu_threads: self.profile_creation_args.per_cpu_threads,
override_arch: None,
unstable_presymbolicate: self.profile_creation_args.unstable_presymbolicate,
}
}
}
Expand Down
12 changes: 12 additions & 0 deletions samply/src/server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use tokio_util::io::ReaderStream;
use wholesym::debugid::DebugId;
use wholesym::{LibraryInfo, SymbolManager, SymbolManagerConfig};

use crate::shared;
use crate::shared::ctrl_c::CtrlC;

#[derive(Clone, Debug)]
Expand Down Expand Up @@ -112,6 +113,16 @@ async fn start_server(
.respect_nt_symbol_path(true)
.use_debuginfod(std::env::var("SAMPLY_USE_DEBUGINFOD").is_ok())
.use_spotlight(true);

if let Some(profile_filename) = profile_filename {
let precog_filename = profile_filename.with_extension("syms.json");
if let Some(precog_info) =
shared::symbol_precog::PrecogSymbolInfo::try_load(&precog_filename)
{
config = config.set_precog_data(precog_info.into_hash_map());
}
}

if let Some(home_dir) = dirs::home_dir() {
config = config.debuginfod_cache_dir_if_not_installed(home_dir.join("sym"));
}
Expand All @@ -124,6 +135,7 @@ async fn start_server(
for lib_info in libinfo_map.into_values() {
symbol_manager.add_known_library(lib_info);
}

let symbol_manager = Arc::new(symbol_manager);

let server = tokio::task::spawn(run_server(
Expand Down
1 change: 1 addition & 0 deletions samply/src/shared/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ pub mod recording_props;
pub mod recycling;
pub mod stack_converter;
pub mod stack_depth_limiting_frame_iter;
pub mod symbol_precog;
pub mod timestamp_converter;
pub mod types;
pub mod unresolved_samples;
Expand Down
Loading

0 comments on commit 81afd7e

Please sign in to comment.