Skip to content

Commit f30c949

Browse files
authored
Improve presymbolication (#549)
Make it respect the various `--symbol-*` command line arguments, and fix a bug caused by incorrect caching.
2 parents 4314b7f + a861f9e commit f30c949

File tree

8 files changed

+256
-199
lines changed

8 files changed

+256
-199
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

samply-symbols/src/mapped_path.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ use nom::{Err, IResult};
1212
/// in [Firefox Breakpad .sym files](https://searchfox.org/mozilla-central/rev/4ebfb48f7e82251145afa4a822f970931dd06c68/toolkit/crashreporter/tools/symbolstore.py#199).
1313
/// This format is also used in the [Tecken symbolication API](https://tecken.readthedocs.io/en/latest/symbolication.html#id9),
1414
/// and [internally in the Firefox Profiler](https://github.com/firefox-devtools/profiler/blob/fb9ff03ab8b98d9e7c29e36314080fae555bbe78/src/utils/special-paths.js).
15-
#[derive(Debug, Clone, PartialEq, Eq)]
15+
#[derive(Debug, Clone, PartialEq, Eq, Hash)]
1616
pub enum MappedPath {
1717
/// A path to a file in a git repository.
1818
Git {

samply-symbols/src/shared.rs

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -455,7 +455,7 @@ pub trait FileContents: Send + Sync {
455455

456456
/// The debug information (function name, file path, line number) for a single frame
457457
/// at the looked-up address.
458-
#[derive(Debug, Clone, PartialEq, Eq)]
458+
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
459459
pub struct FrameDebugInfo {
460460
/// The function name for this frame, if known.
461461
pub function: Option<String>,
@@ -516,7 +516,7 @@ pub trait FileLocation: Clone + Display {
516516
/// refer to a file on this machine or on a different machine (i.e. the original
517517
/// build machine). The mapped path is something like a permalink which potentially
518518
/// allows obtaining the source file from a source server or a public hosted repository.
519-
#[derive(Debug, Clone, PartialEq, Eq)]
519+
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
520520
pub struct SourceFilePath {
521521
/// The raw path to the source file, as written down in the debug file. This is
522522
/// usually an absolute path.
@@ -667,7 +667,7 @@ pub fn relative_address_base<'data>(object_file: &impl object::Object<'data>) ->
667667
}
668668

669669
/// The symbol for a function.
670-
#[derive(Debug, Clone, PartialEq, Eq)]
670+
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
671671
pub struct SymbolInfo {
672672
/// The function's address. This is a relative address.
673673
pub address: u32,
@@ -678,7 +678,7 @@ pub struct SymbolInfo {
678678
}
679679

680680
/// The lookup result for an address.
681-
#[derive(Debug, Clone, PartialEq, Eq)]
681+
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
682682
pub struct AddressInfo {
683683
/// Information about the symbol which contains the looked up address.
684684
pub symbol: SymbolInfo,
@@ -695,7 +695,7 @@ pub struct AddressInfo {
695695
}
696696

697697
/// The lookup result from `lookup_sync`.
698-
#[derive(Debug, Clone, PartialEq, Eq)]
698+
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
699699
pub struct SyncAddressInfo {
700700
/// Information about the symbol which contains the looked up address.
701701
pub symbol: SymbolInfo,
@@ -705,7 +705,7 @@ pub struct SyncAddressInfo {
705705

706706
/// Contains address debug info (inlined functions, file names, line numbers) if
707707
/// available.
708-
#[derive(Debug, Clone, PartialEq, Eq)]
708+
#[derive(Debug, Clone, Hash, PartialEq, Eq)]
709709
pub enum FramesLookupResult {
710710
/// Debug info for this address was found in the symbol map.
711711
///

samply/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ fs4 = "0.13"
5252
humantime = "2.1.0"
5353
shlex = "1.3.0"
5454
samply-quota-manager = { version = "0.1.0", path = "../samply-quota-manager" }
55+
indexmap = "2.9.0"
5556

5657
[target.'cfg(any(target_os = "android", target_os = "macos", target_os = "linux"))'.dependencies]
5758

samply/src/main.rs

Lines changed: 101 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,8 @@ use std::io::BufReader;
2323
use std::path::Path;
2424

2525
use fxprof_processed_profile::Profile;
26+
use shared::ctrl_c::CtrlC;
27+
use wholesym::LibraryInfo;
2628

2729
#[cfg(any(target_os = "android", target_os = "linux"))]
2830
use linux::profiler;
@@ -32,9 +34,10 @@ use mac::profiler;
3234
use windows::profiler;
3335

3436
use profile_json_preparse::parse_libinfo_map_from_profile_file;
35-
use server::start_server_main;
36-
use shared::prop_types::ImportProps;
37+
use server::{start_server, RunningServerInfo, ServerProps};
38+
use shared::prop_types::{ImportProps, SymbolProps};
3739
use shared::save_profile::save_profile_to_file;
40+
use symbols::create_symbol_manager_and_quota_manager;
3841

3942
fn main() {
4043
env_logger::init();
@@ -64,28 +67,10 @@ fn main() {
6467
}
6568

6669
fn do_load_action(load_args: cli::LoadArgs) {
67-
let profile_filename = &load_args.file;
68-
let input_file = match File::open(profile_filename) {
69-
Ok(file) => file,
70-
Err(err) => {
71-
eprintln!("Could not open file {:?}: {}", profile_filename, err);
72-
std::process::exit(1)
73-
}
74-
};
75-
76-
let libinfo_map = match parse_libinfo_map_from_profile_file(input_file, profile_filename) {
77-
Ok(libinfo_map) => libinfo_map,
78-
Err(err) => {
79-
eprintln!("Could not parse the input file as JSON: {}", err);
80-
eprintln!("If this is a perf.data file, please use `samply import` instead.");
81-
std::process::exit(1)
82-
}
83-
};
84-
start_server_main(
85-
profile_filename,
70+
run_server_serving_profile(
71+
&load_args.file,
8672
load_args.server_props(),
8773
load_args.symbol_props(),
88-
libinfo_map,
8974
);
9075
}
9176

@@ -109,21 +94,18 @@ fn do_import_action(import_args: cli::ImportArgs) {
10994
crate::shared::symbol_precog::presymbolicate(
11095
&profile,
11196
&import_args.output.with_extension("syms.json"),
97+
import_args.symbol_props(),
11298
);
11399
}
114100

101+
// Drop the profile so that it doesn't take up memory while the server is running.
102+
drop(profile);
103+
115104
if let Some(server_props) = import_args.server_props() {
116-
let profile_filename = &import_args.output;
117-
let libinfo_map = profile_json_preparse::parse_libinfo_map_from_profile_file(
118-
File::open(profile_filename).expect("Couldn't open file we just wrote"),
119-
profile_filename,
120-
)
121-
.expect("Couldn't parse libinfo map from profile file");
122-
start_server_main(
123-
profile_filename,
105+
run_server_serving_profile(
106+
&import_args.output,
124107
server_props,
125108
import_args.symbol_props(),
126-
libinfo_map,
127109
);
128110
}
129111
}
@@ -155,22 +137,19 @@ fn do_record_action(record_args: cli::RecordArgs) {
155137
crate::shared::symbol_precog::presymbolicate(
156138
&profile,
157139
&record_args.output.with_extension("syms.json"),
140+
record_args.symbol_props(),
158141
);
159142
}
160143

144+
// Drop the profile so that it doesn't take up memory while the server is running.
145+
drop(profile);
146+
161147
// then fire up the server for the profiler front end, if not save-only
162148
if let Some(server_props) = record_args.server_props() {
163-
let profile_filename = &record_args.output;
164-
let libinfo_map = crate::profile_json_preparse::parse_libinfo_map_from_profile_file(
165-
File::open(profile_filename).expect("Couldn't open file we just wrote"),
166-
profile_filename,
167-
)
168-
.expect("Couldn't parse libinfo map from profile file");
169-
start_server_main(
170-
profile_filename,
149+
run_server_serving_profile(
150+
&record_args.output,
171151
server_props,
172152
record_args.symbol_props(),
173-
libinfo_map,
174153
);
175154
}
176155

@@ -227,3 +206,85 @@ fn convert_file_to_profile(
227206
}
228207
}
229208
}
209+
210+
fn run_server_serving_profile(
211+
profile_path: &Path,
212+
server_props: ServerProps,
213+
symbol_props: SymbolProps,
214+
) {
215+
let libinfo_map = {
216+
let profile_file = match File::open(profile_path) {
217+
Ok(file) => file,
218+
Err(err) => {
219+
eprintln!("Could not open file {:?}: {}", profile_path, err);
220+
std::process::exit(1)
221+
}
222+
};
223+
224+
parse_libinfo_map_from_profile_file(profile_file, profile_path)
225+
.expect("Couldn't parse libinfo map from profile file")
226+
};
227+
228+
let runtime = tokio::runtime::Builder::new_multi_thread()
229+
.enable_all()
230+
.build()
231+
.unwrap();
232+
233+
runtime.block_on(async {
234+
let (mut symbol_manager, quota_manager) =
235+
create_symbol_manager_and_quota_manager(symbol_props, server_props.verbose);
236+
for lib_info in libinfo_map.into_values() {
237+
symbol_manager.add_known_library(lib_info);
238+
}
239+
240+
let precog_path = profile_path.with_extension("syms.json");
241+
if let Some(precog_info) = shared::symbol_precog::PrecogSymbolInfo::try_load(&precog_path) {
242+
for (debug_id, syms) in precog_info.into_hash_map().into_iter() {
243+
let lib_info = LibraryInfo {
244+
debug_id: Some(debug_id),
245+
..LibraryInfo::default()
246+
};
247+
symbol_manager.add_known_library_symbols(lib_info, syms);
248+
}
249+
}
250+
251+
let ctrl_c_receiver = CtrlC::observe_oneshot();
252+
253+
let open_in_browser = server_props.open_in_browser;
254+
255+
let RunningServerInfo {
256+
server_join_handle,
257+
server_origin,
258+
profiler_url,
259+
} = start_server(
260+
Some(profile_path),
261+
server_props,
262+
symbol_manager,
263+
ctrl_c_receiver,
264+
)
265+
.await;
266+
267+
eprintln!("Local server listening at {server_origin}");
268+
if !open_in_browser {
269+
if let Some(profiler_url) = &profiler_url {
270+
println!("{profiler_url}");
271+
}
272+
}
273+
eprintln!("Press Ctrl+C to stop.");
274+
275+
if open_in_browser {
276+
if let Some(profiler_url) = &profiler_url {
277+
let _ = opener::open_browser(profiler_url);
278+
}
279+
}
280+
281+
// Run this server until it stops.
282+
if let Err(e) = server_join_handle.await {
283+
eprintln!("server error: {e}");
284+
}
285+
286+
if let Some(quota_manager) = quota_manager {
287+
quota_manager.finish().await;
288+
}
289+
});
290+
}

0 commit comments

Comments
 (0)