Repository navigation
Add --with-samply to "profiler-cli load" - #6339
Conversation
c8d160b to
06ebc3b
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6339 +/- ##
==========================================
+ Coverage 84.03% 84.08% +0.04%
==========================================
Files 354 354
Lines 38100 38287 +187
Branches 10662 10845 +183
==========================================
+ Hits 32018 32194 +176
- Misses 5654 5665 +11
Partials 428 428 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
06ebc3b to
2c6a540
Compare
mstange
left a comment
There was a problem hiding this comment.
Looks fine to me.
One thing we should keep in mind is whether this constrains changes to samply's CLI behavior. For example, if, if we make samply load print the URL differently or if it hands the load off to a daemonized background server and then quits, will this break?Should we check the samply version first and report an error if it's not recognized?
The profiles recorded using the firefox-devtools-mcp are not symbolicated. Also, profiles recorded with "samply record --save-only" need samply's symbol server for symbolication and the assembly view. With `--with-samply` the daemon runs "samply load --no-open <file>", reads the profiler URL it prints, and loads the profile from that URL so the ?symbolServer= parameter is picked up. The samply process is owned by the daemon and is stopped with the session. There are 3 places we look for the `samply` binary: - First we check if `PROFILER_CLI_SAMPLY_PATH` is set. If we so use that samply path directly. - Then we check the `PATH`. - And lastly we check `$MOZBUILD_STATE_PATH/samply` (default ~/.mozbuild/samply), where `mach bootstrap` install it. This is useful for Firefox engineers, so they can use the already preinstalled samply instead of having to install themselves.
The tests use a fake samply fixture with serve, exit, and hang modes that records its pid and the requests it receives, so they do not depend on samply being installed. Binary lookup is covered by unit tests, and the integration tests check that the profile is fetched through samply, that samply is stopped on "stop", on daemon SIGTERM, and when the daemon is stopped while samply is still starting, and that the daemon survives samply dying mid-session.
2c6a540 to
b0aa896
Compare
|
Thanks for the review!
Yeah that's one thing I was thinking about also. I think it would be great if I would rather skip the version check, since it would be a hardcoded allow list that would break with newer versions of samply. I also made 2 small improvements: check both stdout and stderr now (in case it fails), and improved the warnings. |
Changes: [fatadel] Show a category breakdown in profiler-cli (#6256) [fatadel] Show allocation data in the cli (#6246) [Florian Quèze] profiler-cli: name the process in the thread banner, and follow the queried thread (#6269) [fatadel] Drive PII sanitization from marker schemas (#6291) [fatadel] Move FileIO table labels into marker schemas (#6296) [Florian Quèze] profiler-cli: add a thread list command (#6273) [Nazım Can Altınova] Update oxfmt 0.63.0 → 0.68.0 (major) (#6316) [Markus Stange] Add missing finishRawMarkerTableBuilder. (#6318) [Markus Stange] Fix the upload error button contrast (#6315) [Markus Stange] Allow typed arrays in the nativeSymbols table (#6300) [Markus Stange] Remove brittle test. (#6322) [Markus Stange] Fix file extension typo. (#6325) [Nazım Can Altınova] Add a permalink command to profiler-cli for already published profiles (#6331) [Markus Stange] Typed arrays + flags for the FuncTable (#6323) [Florian Quèze] profiler-cli: add profile markers for cross-thread marker search (#6276) [fatadel] Convert extension text markers to structured payloads (#6314) [Florian Quèze] profiler-cli: include per-marker fields and data in --list --json (#6275) [Nazım Can Altınova] Add `--with-samply` to "profiler-cli load" (#6339) [Markus Stange] Speed up call tree sidebar by sharing work with the activity graph (#6329) [Nazım Can Altınova] 🔃 Sync: l10n -> main (Sept 22, 2026) (#6341) [Nazım Can Altınova] Bump profiler-cli version to 0.10.0 (#6342) And special thanks to our localizers: el: Jim Spentzos ro: robbp
Main | Deploy preview
Fixes #6337.
The profiles recorded using the firefox-devtools-mcp are not symbolicated. Also, profiles recorded with "samply record --
save-only" need samply's symbol server for symbolication and the assembly view.
With
--with-samplythe daemon runs "samply load --no-open ", reads the profiler URL it prints, and loads the profile from that URL so the ?symbolServer= parameter is picked up. The samply process is owned by the daemon and is stopped with the session.There are 3 places we look for the
samplybinary:PROFILER_CLI_SAMPLY_PATHis set. If we so use that samply path directly.PATH.$MOZBUILD_STATE_PATH/samply(default~/.mozbuild/samply), wheremach bootstrapinstall it. This is useful for Firefox engineers, so they can use the already preinstalled samply instead of having to install themselves.