Skip to content

chore/revert samply bump - #552

Open
not-matthias wants to merge 1 commit into
mainfrom
chore/revert-samply-bump
Open

not-matthias wants to merge 1 commit into
mainfrom
chore/revert-samply-bump

Conversation

@not-matthias

@not-matthias not-matthias commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

It tried to fix an issue in flamegraphs with wrong attributed callchains, but breaks recursive/deep flamegraphs.

@not-matthias
not-matthias marked this pull request as ready for review September 28, 2026 14:40
@greptile-apps

greptile-apps Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 4/5

[High risk] Reverts a dependency to an earlier commit.

The PR does not appear safe to merge while the default profiler breaks recursive and deep flamegraphs.

Fix All in Claude CodeFindings

  1. P1 Recursive flamegraphs break ▶
Fix with agent prompt
### Issue 1
crates/samply-codspeed:1
This revert restores the Samply revision that, as the PR description notes, breaks recursive and deep flamegraphs. Samply is still the default wall-time profiler, so benchmarks with those call stacks will produce broken flamegraphs even though callchain attribution improves.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

---

For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.

Summary

The PR reverts the pinned Samply submodule revision to address incorrectly attributed flamegraph callchains.

  • Samply is the default wall-time profiler.
  • The PR description identifies a resulting regression for recursive and deep flamegraphs.

Reviews (1) · Last reviewed commit: "Revert "chore: bump samply-codspeed""

Comment thread crates/samply-codspeed
@codspeed

codspeed Bot commented Sep 28, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 13.82%

⚠️ Unknown Walltime execution environment detected

Using the Walltime instrument on standard Hosted Runners will lead to inconsistent data.

For the most accurate results, we recommend using CodSpeed Macro Runners: bare-metal machines fine-tuned for performance measurement consistency.

❌ 1 regressed benchmark
✅ 32 untouched benchmarks
⏩ 4 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
❌ WallTime memtrack track tar 8 s 9.3 s -13.82%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing chore/revert-samply-bump (98ef901) with main (7c135cc)

Open in CodSpeed

Footnotes

  1. 4 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports. ↩

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.

2 participants