Skip to content

Weight results by perf event periods - #119

Draft
antiguru wants to merge 20 commits into
shared-tables-formatfrom
period-weighting
Draft

antiguru wants to merge 20 commits into
shared-tables-formatfrom
period-weighting

Conversation

@antiguru

@antiguru antiguru commented Sep 26, 2026 •

Copy link
Copy Markdown
Owner

pollard counted every sample and every perf event marker as 1. In frequency mode (perf record -F), perf varies the period per sample, so count shares are biased. samply can now record each event's period and name the events in a Perf events section of meta.extra, with sample weights under samply import --weight-by-period.

pollard parses that section and weights top_functions, call_tree, stacks_containing, folded_stacks, top_groups, compare_profiles, compare_functions, the source and asm listings, and the summary rankings by period. Each of these reports a weighted flag, and compare_profiles adds a note when only one side is weighted. Totals in summary and describe_profile stay raw counts, compare_profiles millisecond columns stay count-based, and list_events names the main event and each event's sampling. Profiles without period information produce the same output as before, apart from the new flags.

Stacked on #118. The samply side is mstange/samply#888 (periods and event metadata) and mstange/samply#889 (CPU delta for clock events only). Spec and plan are in docs/superpowers/.

Posted by Claude Code.

🤖 Generated with Claude Code

https://claude.ai/code/session_016aEc8uf7GZbHQHXmoX8o2k

antiguru and others added 20 commits September 26, 2026 13:03
Cover all main-event sample sites, zero off-CPU weights under
--weight-by-period, keep compare_profiles _ms count-based, define the
weighted flag and the Perf events grammar, and make the samply weight
logic unit-testable.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016aEc8uf7GZbHQHXmoX8o2k
Record the sample weight mode in the Perf events section, keep the two
samply pull requests independent, weight summary rankings, and spell
out accumulators, RawMeta.extra, and exact test comparisons.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016aEc8uf7GZbHQHXmoX8o2k
Also settle two spec gaps the plan surfaced: attributes that do not
sample get `no sampling`, and a marker period of 0 falls back to the
event's fixed period.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016aEc8uf7GZbHQHXmoX8o2k
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016aEc8uf7GZbHQHXmoX8o2k
aggregate_grouped's doc paragraph still pointed at the removed
event::stack_indices helper and its old Option<usize> yield shape.
compare_profiles's delta computations used `weight as i64`; switch to
i64::try_from with a saturating fallback.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016aEc8uf7GZbHQHXmoX8o2k
Extract build_listing (asm.rs) and build_output (compare_functions.rs)
sync tails so weighted-flag tests use hand-built decoded/listing data
instead of depending on the test binary's own layout at a hardcoded
address, which isn't guaranteed to decode and doesn't hold on macOS.
Also adds a source.rs regression test for reading sample weights by
original (not compacted) index across a null-stack sample.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016aEc8uf7GZbHQHXmoX8o2k
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016aEc8uf7GZbHQHXmoX8o2k
Marker events are always weighted by their perf event period, but the
samples track only counts events when --weight-by-period is passed.
A fixed period (-c N) does not make the two comparable on its own; it
was wrong to say it gives exact cross-event comparisons without the
flag.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016aEc8uf7GZbHQHXmoX8o2k
…JSLB

samply's default output is .jslb.gz, but the only period-weighted
end-to-end fixture was JSON. Extend regenerate.sh to also import the
recording with the period-support samply build as
multi_period.jslb.gz, scrubbed like the other fixtures. Regenerating
from a single new recording refreshes every existing fixture, so
multi_v49/multi_v75 change too. All four surviving cross-fixture tests
still pass.

Add a test asserting multi_period.json.gz and multi_period.jslb.gz
give identical top_functions output and weighted flags across the
samples track and every marker event, modeled on
json_and_jslb_from_one_build_match.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016aEc8uf7GZbHQHXmoX8o2k
Tally::add/merge in top_functions.rs already saturate on overflow, but
call_tree, folded, stacks_containing, and summary accumulated
per-event weights with plain +=. Switch those weight accumulations to
saturating_add so every tool follows the same overflow policy. No
behavior change for realistic values.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_016aEc8uf7GZbHQHXmoX8o2k
@antiguru
antiguru added this pull request to stack #120 September 26, 2026 13:50
@antiguru antiguru changed the title period weighting Weight results by perf event periods Sep 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant