Skip to content

Commit 99c1190

Browse files
authored
perf(profiling): drop unnecessary mapping ID (#925)
* perf(profiling): drop unnecessary mapping ID This allows Location.mapping_id=0 in the final pprof, and since that's the default for the field, it can be omitted altogether from the profile. This shrinks every location entry on-wire by a few bytes before compression. * perf(profiling): optimize for zero-mapping
1 parent cde6b43 commit 99c1190

File tree

3 files changed

+54
-9
lines changed

3 files changed

+54
-9
lines changed

profiling/src/internal/location.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use super::*;
1010
/// struct.
1111
#[derive(Copy, Clone, Debug, Eq, PartialEq, Hash, PartialOrd, Ord)]
1212
pub struct Location {
13-
pub mapping_id: MappingId,
13+
pub mapping_id: Option<MappingId>,
1414
pub function_id: FunctionId,
1515
pub address: u64,
1616
pub line: i64,
@@ -26,7 +26,7 @@ impl PprofItem for Location {
2626
fn to_pprof(&self, id: Self::Id) -> Self::PprofMessage {
2727
pprof::Location {
2828
id: id.to_raw_id(),
29-
mapping_id: self.mapping_id.to_raw_id(),
29+
mapping_id: self.mapping_id.map(MappingId::into_raw_id).unwrap_or(0),
3030
address: self.address,
3131
lines: vec![pprof::Line {
3232
function_id: self.function_id.to_raw_id(),

profiling/src/internal/profile/fuzz_tests.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -277,7 +277,13 @@ fn assert_samples_eq(
277277
// `small_non_zero_pprof_id()` function which guarantees that the id stored in pprof
278278
// is +1 of the index in the vector of Locations in internal::Profile.
279279
let location = &profile.locations[*loc_id as usize - 1];
280-
let mapping = &profile.mappings[location.mapping_id as usize - 1];
280+
281+
// PHP, Python, and Ruby don't use mappings, so allow for zero id.
282+
let mapping = if location.mapping_id != 0 {
283+
profile.mappings[location.mapping_id as usize - 1]
284+
} else {
285+
Default::default()
286+
};
281287
// internal::Location::to_pprof() always creates a single line.
282288
assert!(location.lines.len() == 1);
283289
let line = location.lines[0];

profiling/src/internal/profile/mod.rs

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -467,33 +467,72 @@ impl Profile {
467467
})
468468
}
469469

470-
fn add_mapping(&mut self, mapping: &api::Mapping) -> MappingId {
470+
fn add_mapping(&mut self, mapping: &api::Mapping) -> Option<MappingId> {
471+
#[inline]
472+
fn is_zero_mapping(mapping: &api::Mapping) -> bool {
473+
// - PHP, Python, and Ruby use a mapping only as required.
474+
// - .NET uses only the filename.
475+
// - The native profiler uses all fields.
476+
// We strike a balance for optimizing for the dynamic languages
477+
// and the others by mixing branches and branchless programming.
478+
let filename = mapping.filename.len();
479+
let build_id = mapping.build_id.len();
480+
if 0 != (filename | build_id) {
481+
return false;
482+
}
483+
484+
let memory_start = mapping.memory_start;
485+
let memory_limit = mapping.memory_limit;
486+
let file_offset = mapping.file_offset;
487+
0 == (memory_start | memory_limit | file_offset)
488+
}
489+
490+
if is_zero_mapping(mapping) {
491+
return None;
492+
}
493+
471494
let filename = self.intern(mapping.filename);
472495
let build_id = self.intern(mapping.build_id);
473496

474-
self.mappings.dedup(Mapping {
497+
Some(self.mappings.dedup(Mapping {
475498
memory_start: mapping.memory_start,
476499
memory_limit: mapping.memory_limit,
477500
file_offset: mapping.file_offset,
478501
filename,
479502
build_id,
480-
})
503+
}))
481504
}
482505

483506
fn add_string_id_mapping(
484507
&mut self,
485508
mapping: &api::StringIdMapping,
486-
) -> anyhow::Result<MappingId> {
509+
) -> anyhow::Result<Option<MappingId>> {
510+
#[inline]
511+
fn is_zero_mapping(mapping: &api::StringIdMapping) -> bool {
512+
// See the other is_zero_mapping for more info, but only Ruby is
513+
// using this API at the moment, so we optimize for the whole
514+
// thing being a zero representation.
515+
let memory_start = mapping.memory_start;
516+
let memory_limit = mapping.memory_limit;
517+
let file_offset = mapping.file_offset;
518+
let strings = (mapping.filename.value | mapping.build_id.value) as u64;
519+
0 == (memory_start | memory_limit | file_offset | strings)
520+
}
521+
522+
if is_zero_mapping(mapping) {
523+
return Ok(None);
524+
}
525+
487526
let filename = self.resolve(mapping.filename)?;
488527
let build_id = self.resolve(mapping.build_id)?;
489528

490-
Ok(self.mappings.dedup(Mapping {
529+
Ok(Some(self.mappings.dedup(Mapping {
491530
memory_start: mapping.memory_start,
492531
memory_limit: mapping.memory_limit,
493532
file_offset: mapping.file_offset,
494533
filename,
495534
build_id,
496-
}))
535+
})))
497536
}
498537

499538
fn add_stacktrace(&mut self, locations: Vec<LocationId>) -> StackTraceId {

0 commit comments

Comments
 (0)