Skip to content

Commit abe75d8

Browse files
committed
Share Vec<SourceLine> and Vec<Inlinee> among parsed breakpad functions.
This reduces the time to drop the SymbolMap because there are fewer small allocations to free. Before: https://share.firefox.dev/3JYDSFP After: https://share.firefox.dev/3V6rciL
1 parent 502b740 commit abe75d8

File tree

2 files changed

+65
-30
lines changed

2 files changed

+65
-30
lines changed

samply-symbols/src/breakpad/index.rs

Lines changed: 53 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -401,29 +401,40 @@ pub struct BreakpadFuncSymbol {
401401
}
402402

403403
impl BreakpadFuncSymbol {
404-
pub fn parse(mut input: &[u8]) -> Result<BreakpadFuncSymbolInfo<'_>, BreakpadParseError> {
404+
pub fn parse<'a, 'b>(
405+
mut input: &'a [u8],
406+
lines: &'b mut Vec<SourceLine>,
407+
inlinees: &'b mut Vec<Inlinee>,
408+
) -> Result<BreakpadFuncSymbolInfo<'a>, BreakpadParseError> {
405409
let first_line = read_line_and_advance(&mut input);
406410
let (_address, size, name) =
407411
func_line(first_line).map_err(|_| BreakpadParseError::ParsingFunc)?;
408412

413+
let lines_start_index = lines.len();
414+
let inlinees_start_index = inlinees.len();
415+
409416
let mut tokenizer = Tokenizer::new(input);
410-
let mut inlinees = Vec::new();
411-
let mut lines = Vec::new();
412417
while !tokenizer.eof() {
413418
if tokenizer.consume_token(b"INLINE").is_ok() {
414-
parse_inline_line_remainder(&mut tokenizer, &mut inlinees)
419+
parse_inline_line_remainder(&mut tokenizer, inlinees)
415420
.map_err(|_| BreakpadParseError::ParsingInline)?;
416421
} else if let Ok(line_data) = parse_func_data_line(&mut tokenizer) {
417422
lines.push(line_data);
418423
}
419424
tokenizer.consume_until_after_next_line_break_or_eof();
420425
}
421-
inlinees.sort_unstable_by_key(|inlinee| (inlinee.depth, inlinee.address));
426+
427+
let lines_end_index = lines.len();
428+
let inlinees_end_index = inlinees.len();
429+
430+
inlinees[inlinees_start_index..inlinees_end_index]
431+
.sort_unstable_by_key(|inlinee| (inlinee.depth, inlinee.address));
432+
422433
Ok(BreakpadFuncSymbolInfo {
423434
name: str::from_utf8(name).map_err(|_| BreakpadParseError::BadUtf8)?,
424435
size,
425-
lines,
426-
inlinees,
436+
line_index_range: (lines_start_index as u32, lines_end_index as u32),
437+
inlinee_index_range: (inlinees_start_index as u32, inlinees_end_index as u32),
427438
})
428439
}
429440
}
@@ -782,15 +793,25 @@ pub struct BreakpadPublicSymbolInfo<'a> {
782793
pub name: &'a str,
783794
}
784795

785-
#[derive(Debug, Clone)]
796+
#[derive(Debug, Clone, Copy)]
786797
pub struct BreakpadFuncSymbolInfo<'a> {
787798
pub name: &'a str,
788799
pub size: u32,
789-
pub lines: Vec<SourceLine>,
790-
pub inlinees: Vec<Inlinee>,
800+
pub line_index_range: (u32, u32),
801+
pub inlinee_index_range: (u32, u32),
791802
}
792803

793804
impl BreakpadFuncSymbolInfo<'_> {
805+
pub fn lines<'a>(&self, lines: &'a [SourceLine]) -> &'a [SourceLine] {
806+
let (s, e) = self.line_index_range;
807+
&lines[s as usize..e as usize]
808+
}
809+
810+
pub fn inlinees<'a>(&self, inlinees: &'a [Inlinee]) -> &'a [Inlinee] {
811+
let (s, e) = self.inlinee_index_range;
812+
&inlinees[s as usize..e as usize]
813+
}
814+
794815
/// Returns `(file_id, line, address)` of the line record that covers the
795816
/// given address. Line records describe locations at the deepest level of
796817
/// inlining at that address.
@@ -799,13 +820,14 @@ impl BreakpadFuncSymbolInfo<'_> {
799820
/// address, i.e. both the call to B and the call to C have been inlined all
800821
/// the way into A (A being the "outer function"), then this method reports
801822
/// locations in C.
802-
pub fn get_innermost_sourceloc(&self, addr: u32) -> Option<&SourceLine> {
803-
let line_index = match self.lines.binary_search_by_key(&addr, |line| line.address) {
823+
pub fn get_innermost_sourceloc(&self, addr: u32, lines: &[SourceLine]) -> Option<SourceLine> {
824+
let lines = self.lines(lines);
825+
let line_index = match lines.binary_search_by_key(&addr, |line| line.address) {
804826
Ok(i) => i,
805827
Err(0) => return None,
806828
Err(i) => i - 1,
807829
};
808-
Some(&self.lines[line_index])
830+
Some(lines[line_index])
809831
}
810832

811833
/// Returns `(call_file_id, call_line, address, inline_origin)` of the
@@ -815,22 +837,27 @@ impl BreakpadFuncSymbolInfo<'_> {
815837
/// A -> B -> C at an address, i.e. both the call to B and the call to C have
816838
/// been inlined all the way into A (A being the "outer function"), then the
817839
/// call A -> B is at level zero, and the call B -> C is at level one.
818-
pub fn get_inlinee_at_depth(&self, depth: u32, addr: u32) -> Option<&Inlinee> {
819-
let index = match self
820-
.inlinees
840+
pub fn get_inlinee_at_depth(
841+
&self,
842+
depth: u32,
843+
addr: u32,
844+
inlinees: &[Inlinee],
845+
) -> Option<Inlinee> {
846+
let inlinees = self.inlinees(inlinees);
847+
let index = match inlinees
821848
.binary_search_by_key(&(depth, addr), |inlinee| (inlinee.depth, inlinee.address))
822849
{
823850
Ok(i) => i,
824851
Err(0) => return None,
825852
Err(i) => i - 1,
826853
};
827-
let inlinee = &self.inlinees[index];
854+
let inlinee = &inlinees[index];
828855
if inlinee.depth != depth {
829856
return None;
830857
}
831858
let end_address = inlinee.address.checked_add(inlinee.size)?;
832859
if addr < end_address {
833-
Some(inlinee)
860+
Some(*inlinee)
834861
} else {
835862
None
836863
}
@@ -952,7 +979,7 @@ fn public_line(input: &[u8]) -> IResult<&[u8], (u32, &[u8])> {
952979
}
953980

954981
/// A mapping from machine code bytes to source line and file.
955-
#[derive(Clone, Debug, PartialEq, Eq)]
982+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
956983
pub struct SourceLine {
957984
/// The start address relative to the module's load address.
958985
pub address: u32,
@@ -967,7 +994,7 @@ pub struct SourceLine {
967994
}
968995

969996
/// A single range which is covered by an inlined function call.
970-
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
997+
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
971998
pub struct Inlinee {
972999
/// The depth of the inline call.
9731000
pub depth: u32,
@@ -1204,12 +1231,14 @@ mod test {
12041231
block_length: (block.len() - "JUNK\n".len() - "\nJUNK".len()) as u32,
12051232
};
12061233
let input = &block[func.file_offset as usize..][..func.block_length as usize];
1207-
let func = BreakpadFuncSymbol::parse(input).unwrap();
1234+
let mut lines = Vec::new();
1235+
let mut inlinees = Vec::new();
1236+
let func = BreakpadFuncSymbol::parse(input, &mut lines, &mut inlinees).unwrap();
12081237
assert_eq!(func.name, "main");
12091238
assert_eq!(func.size, 0x28);
1210-
assert_eq!(func.lines.len(), 4);
1239+
assert_eq!(func.lines(&lines).len(), 4);
12111240
assert_eq!(
1212-
func.lines[0],
1241+
func.lines(&lines)[0],
12131242
SourceLine {
12141243
address: 0x1130,
12151244
size: 0xf,
@@ -1218,7 +1247,7 @@ mod test {
12181247
}
12191248
);
12201249
assert_eq!(
1221-
func.lines[3],
1250+
func.lines(&lines)[3],
12221251
SourceLine {
12231252
address: 0x114f,
12241253
size: 0x9,

samply-symbols/src/breakpad/symbol_map.rs

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ use super::index::{
1313
BreakpadPublicSymbol, BreakpadPublicSymbolInfo, FileOrInlineOrigin, SYMBOL_ENTRY_KIND_FUNC,
1414
SYMBOL_ENTRY_KIND_PUBLIC,
1515
};
16+
use crate::breakpad::index::{Inlinee, SourceLine};
1617
use crate::symbol_map::{GetInnerSymbolMap, SymbolMapTrait};
1718
use crate::{
1819
Error, FileContents, FileContentsWrapper, FrameDebugInfo, FramesLookupResult, LookupAddress,
@@ -131,6 +132,8 @@ struct BreakpadSymbolMapCache<'a, T: FileContents> {
131132
struct BreakpadSymbolMapSymbolCache<'a> {
132133
public_symbols: HashMap<u64, BreakpadPublicSymbolInfo<'a>>,
133134
func_symbols: HashMap<u64, BreakpadFuncSymbolInfo<'a>>,
135+
lines: Vec<SourceLine>,
136+
inlinees: Vec<Inlinee>,
134137
}
135138

136139
impl<'a, T: FileContents> BreakpadSymbolMapCache<'a, T> {
@@ -169,17 +172,17 @@ impl<'a> BreakpadSymbolMapSymbolCache<'a> {
169172
file_offset: u64,
170173
block_length: u32,
171174
data: &'a FileContentsWrapper<T>,
172-
) -> Result<&'s BreakpadFuncSymbolInfo<'a>, Error> {
175+
) -> Result<BreakpadFuncSymbolInfo<'a>, Error> {
173176
match self.func_symbols.entry(file_offset) {
174-
Entry::Occupied(info) => Ok(info.into_mut()),
177+
Entry::Occupied(info) => Ok(*info.get()),
175178
Entry::Vacant(vacant) => {
176179
let block = data
177180
.read_bytes_at(file_offset, block_length.into())
178181
.map_err(|e| {
179182
Error::HelperErrorDuringFileReading("Breakpad FUNC symbol".to_string(), e)
180183
})?;
181-
let info = BreakpadFuncSymbol::parse(block)?;
182-
Ok(vacant.insert(info))
184+
let info = BreakpadFuncSymbol::parse(block, &mut self.lines, &mut self.inlinees)?;
185+
Ok(*vacant.insert(info))
183186
}
184187
}
185188
}
@@ -324,6 +327,7 @@ impl<T: FileContents> SymbolMapTrait for BreakpadSymbolMapInner<'_, T> {
324327
let info = symbols
325328
.get_func_info(offset, line_or_block_len, self.data)
326329
.ok()?;
330+
let symbols = &*symbols;
327331
let func_end_addr = symbol_address + info.size;
328332
if address >= func_end_addr {
329333
return None;
@@ -332,7 +336,9 @@ impl<T: FileContents> SymbolMapTrait for BreakpadSymbolMapInner<'_, T> {
332336
let mut frames = Vec::new();
333337
let mut depth = 0;
334338
let mut name = Some(info.name);
335-
while let Some(inlinee) = info.get_inlinee_at_depth(depth, address) {
339+
while let Some(inlinee) =
340+
info.get_inlinee_at_depth(depth, address, &symbols.inlinees)
341+
{
336342
let file = files.get_string(inlinee.call_file).ok();
337343
frames.push(FrameDebugInfo {
338344
function: name.map(ToString::to_string),
@@ -344,7 +350,7 @@ impl<T: FileContents> SymbolMapTrait for BreakpadSymbolMapInner<'_, T> {
344350
name = inline_origin;
345351
depth += 1;
346352
}
347-
let line_info = info.get_innermost_sourceloc(address);
353+
let line_info = info.get_innermost_sourceloc(address, &symbols.lines);
348354
let (file, line_number) = if let Some(line_info) = line_info {
349355
let file = files.get_string(line_info.file).ok();
350356
(file, Some(line_info.line))

0 commit comments

Comments
 (0)