perf(debugger): sample probes in breakpoint conditions#8967
Merged
Conversation
Collaborator
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
github-actions
Bot
added
semver-patch
debugger
Contributor
Overall package sizeSelf size: 6.36 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.2.0 | 104.26 kB | 843.44 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | dc-polyfill | 0.1.11 | 25.74 kB | 25.74 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
🎉 All green!🧪 All tests passed 🔗 Commit SHA: d869c02 | Docs | Datadog PR Page | Give us feedback! |
watson
force-pushed
the
graphite-base/8967
branch
from
June 17, 2026 19:53
4776de7 to
eb0fd43
Compare
watson
force-pushed
the
watson/DEBUG-5778/refactor-sampling
branch
from
June 17, 2026 19:53
7f047cd to
bc04cb3
Compare
watson
changed the base branch from
graphite-base/8967
to
watson/remove-dead-code
June 17, 2026 19:53
watson
force-pushed
the
watson/DEBUG-5778/refactor-sampling
branch
3 times, most recently
from
June 18, 2026 05:30
9aa4315 to
63aa918
Compare
BenchmarksBenchmark execution time: 2026-06-26 16:56:26 Comparing candidate commit d869c02 in PR branch Found 1 performance improvements and 0 performance regressions! Performance is the same for 2222 metrics, 63 unstable metrics.
|
watson
force-pushed
the
watson/DEBUG-5778/refactor-sampling
branch
from
June 18, 2026 12:03
63aa918 to
08a01ab
Compare
watson
force-pushed
the
watson/remove-dead-code
branch
from
June 18, 2026 12:03
eb0fd43 to
5793b97
Compare
watson
force-pushed
the
watson/DEBUG-5778/refactor-sampling
branch
2 times, most recently
from
June 18, 2026 13:49
5b3deae to
fa6cdfa
Compare
watson
force-pushed
the
watson/remove-dead-code
branch
from
June 18, 2026 13:49
5793b97 to
e2e691d
Compare
watson
added a commit
that referenced
this pull request
Jun 18, 2026
The probe sampling refactor in #8967 moved sampled probe tracking into the sampler buffer, but left behind the old snapshot probe index bookkeeping. Remove the dead resizeable ArrayBuffer path and Node.js version gate from the paused debugger handler.
watson
force-pushed
the
watson/DEBUG-5778/refactor-sampling
branch
from
June 18, 2026 15:45
fa6cdfa to
6894af5
Compare
watson
marked this pull request as ready for review
June 18, 2026 15:46
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6894af5eed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
tlhunter
pushed a commit
that referenced
this pull request
Jun 18, 2026
The probe sampling refactor in #8967 moved sampled probe tracking into the sampler buffer, but left behind the old snapshot probe index bookkeeping. Remove the dead resizeable ArrayBuffer path and Node.js version gate from the paused debugger handler.
watson
force-pushed
the
watson/DEBUG-5778/refactor-sampling
branch
2 times, most recently
from
June 19, 2026 11:57
1f351f5 to
75ba72e
Compare
grantseltzer
previously approved these changes
Jun 19, 2026
BridgeAR
previously approved these changes
Jun 23, 2026
BridgeAR
left a comment
BridgeAR
left a comment
Member
There was a problem hiding this comment.
Nice, this is looking really great!
I have a few small nits, nothing blocking. The only question is about cross realm probes.
The cross realm aspect we are loosing is something I am uncertain about. Should we just keep the old approach when cross realm breakpoints are used? We can detect these, if I am not mistaken and could handle them accordingly.
watson
force-pushed
the
watson/DEBUG-5778/refactor-sampling
branch
from
June 25, 2026 09:30
75ba72e to
3f928f1
Compare
BridgeAR
previously approved these changes
Jun 25, 2026
BridgeAR
left a comment
BridgeAR
left a comment
Member
There was a problem hiding this comment.
LGTM, while I still believe it would be nice to keep the old logic as fallback for cross realm probes
tylfin
previously approved these changes
Jun 26, 2026
Move probe sampling into generated breakpoint conditions so unsampled hits can avoid pausing the application. The devtools worker now reads sampled probe indexes from a shared buffer instead of evaluating conditions and sampling after every pause. Install a runtime sampler in the debuggee, track probe indexes in worker state, and clear sampler state when probes are removed. This keeps per-probe sampling, user conditions, and the global snapshot sampling limit in the condition path. Add focused coverage for sampler expressions, breakpoint condition rebuilding, pause handling, overflow handling, and stale sampled probe indexes.
watson
force-pushed
the
watson/DEBUG-5778/refactor-sampling
branch
from
June 26, 2026 16:34
3f928f1 to
d869c02
Compare
tylfin
approved these changes
Jun 26, 2026
watson
enabled auto-merge (squash)
June 26, 2026 16:38
Merged
pabloerhard
pushed a commit
that referenced
this pull request
Jun 26, 2026
The probe sampling refactor in #8967 moved sampled probe tracking into the sampler buffer, but left behind the old snapshot probe index bookkeeping. Remove the dead resizeable ArrayBuffer path and Node.js version gate from the paused debugger handler.
pabloerhard
pushed a commit
that referenced
this pull request
Jun 26, 2026
Move probe sampling into generated breakpoint conditions so unsampled hits can avoid pausing the application. The devtools worker now reads sampled probe indexes from a shared buffer instead of evaluating conditions and sampling after every pause. Install a runtime sampler in the debuggee from the debugger bootstrap, hand the shared buffer to the worker, and uninstall it on cleanup so the sampler survives debugger stop/start. Track probe indexes in worker state and clear per-probe sampler state when a probe is removed. This keeps per-probe sampling, user conditions, and the global snapshot sampling limit in the condition path. Rework the sirun debugger benchmark to exercise the new condition path: add a second breakpoint target, make the startup-guard share and operation counts configurable, drop the install-only variant, and honor per-variant iteration counts in the overview collector. Add focused coverage for sampler expressions, breakpoint condition rebuilding, pause handling, overflow handling, stop/start, and stale sampled probe indexes.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Moves Node.js debugger probe sampling into generated breakpoint conditions so unsampled probe hits can avoid pausing the debuggee process. The debugger bootstrap installs a runtime sampler in the debuggee process and shares a buffer with the devtools worker. Breakpoint conditions make the sampling decision inline and record sampled probe indexes into that buffer; the worker then only processes the probes the condition path sampled.
Motivation
Probe sampling was previously decided after V8 had already paused the application and control had moved to the devtools worker. That meant probes dropped by per-probe sampling or the global snapshot sampling limit could still add pause overhead. Moving the decision into the breakpoint condition makes the common unsampled case cheaper.
Additional Notes
Added focused tests for
probe_sampler, breakpoint condition rebuilding, sampler state cleanup on probe removal, pause handling with sampled probe indexes, overflow handling, and stale/inconsistent sampled probe state.Known limitations
Breakpoint conditions read the sampler from the realm-local
globalThis, so a probe whose code runs in a separate V8 realm (e.g. avm.runInContextscript) can't reach it and silently won't fire, rather than crash.In practice the only affected user is one who runs application code in a vm context with an absolute-path filename and places a probe on a line inside it:
This only narrows an already-degraded, undocumented path for Dynamic Instrumentation: even before this change such probes produced correlation-less snapshots, because
global.requiredoesn't exist in the vm realm. Test Optimization's failing-test replay is unaffected, as it runs in a separate worker that sets unconditional breakpoints.