Skip to content

gh-154090: Store profiling mode in binary files#154105

Open
pablogsal wants to merge 2 commits into
python:mainfrom
pablogsal:agent/gh-154090-binary-mode
Open

gh-154090: Store profiling mode in binary files#154105
pablogsal wants to merge 2 commits into
python:mainfrom
pablogsal:agent/gh-154090-binary-mode

Conversation

@pablogsal

@pablogsal pablogsal commented Jul 19, 2026

Copy link
Copy Markdown
Member

@pablogsal pablogsal added the needs backport to 3.15 pre-release feature fixes, bugs and security fixes label Jul 19, 2026
@pablogsal

Copy link
Copy Markdown
Member Author

CC: @lkollar

@pablogsal
pablogsal marked this pull request as ready for review July 19, 2026 14:08
@pablogsal
pablogsal requested a review from AA-Turner as a code owner July 19, 2026 14:08

@maurycy maurycy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about threads (ie: main v. --all-threads; now implicit from HDR_OFF_THREADS), native_frames, gc for diffs?

Truth be told, I wonder about the all flags (especially if we strictly care about honest diffs and replays), about describing the target (especially for Gecko), but also about error rate, maybe even failed samples (ref #154092).

That's a bit of a Whac-A-Mole without approaching this is in a bit more generic way (eg: flexible metadata struct within the existing header.)

| 44 | 8 | uint64 | Frame table offset |
| 52 | 4 | uint32 | Compression type (0=none, 1=zstd) |
| 56 | 8 | bytes | Reserved (zero-filled) |
| 28 | 8 | uint64 | Total sample count |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you

Comment on lines +94 to +95
Profiling modes use their `_remote_debugging.PROFILING_MODE_*` value plus one,
leaving zero available for binaries written before this field was defined.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we care about backwards compatibility that much, given that we have recently changed the total sample count from uint32 to uint64 lately (#153425)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

awaiting core review needs backport to 3.15 pre-release feature fixes, bugs and security fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Sampling binary profiles do not record the profiling mode

2 participants