fix: paths and time are two unwinders on one stack, and cannot both run - #23
Merged
Merged
Conversation
`record` crashed. Not "returned a wrong number" — signal 11, no Julia backtrace, roughly one full
suite run in five. It reached CI twice before it was caught, and both times the expression it died
in was in `test_spec_profile.jl`'s merge region, a few lines after the threaded record.
The cause is a **conjunction**, and each half was measured alone before the pair was refused.
Capturing a call path calls `backtrace()`, which unwinds the calling thread; the timing backend's
sampler unwinds those same threads from outside. Two unwinders on one stack. 1.12.7, 150 threaded
records per run:
paths alone 0/9 crashed
timing alone 0/4
both 2/4 segmentation fault, no Julia backtrace
The first stress written for this missed it: it ran 600 records on `-t4` but the *work* inside
them was single-threaded, so `backtrace()` was only ever called from the thread the sampler was
least likely to be walking. Adding `Threads.@threads` to the recorded block is what made it a
measurement instead of a guess.
So the pair is refused rather than risked, and the refusal names the measurement rather than
saying "unsupported". The default moves to `paths = true, timing = false` — the instrument that
needs no sampler, and therefore has no global side effect on `Profile`'s sampling interval either.
`record(f; paths = false, timing = true)` is the other one.
The spec's timing claims now pass that keyword explicitly, and a new testset pins the refusal with
both single-instrument calls as controls: a refusal of the PAIR is not a refusal of either half,
and without the controls those two readings are the same test.
Nothing else in the package asked for both: `stamp` takes the default, `assert_clean` asks for
neither.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
📚 Docs preview: https://codes.sota-shimozono.com/ExperimentalAPI.jl/previews/PR23/ (updates on each push to this PR) |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
sotashimozono
added a commit
that referenced
this pull request
Sep 6, 2026
) `entered()` answers this about a whole process and `record(() -> f(x))` about one call. The macro earns its place by knowing two things a closure cannot: the source text of the expression, and the line it was written on. julia> ExperimentalAPI.@entered sweep(model; βs = 0.05:0.05:2.0) ┌ @entered sweep(model; βs = 0.05:0.05:2.0) at sweep.jl:42 │ MyPkg.energy ×10000 — convergence not established below β ≈ 0.1 │ MyPkg.correlator × 500 — edge cases at zero separation untested └ 15 of 17 observable marked definitions were not entered 0.42713… It returns the value of the expression, so it drops into existing code the way `@time` does. The last line is the point: "entered nothing" and "nothing is marked anywhere" are different states, and a package that has not adopted this yet is in the second one. Pinned by tests that nothing else would catch: the expression is evaluated **exactly once** (verified by mutation — splicing it twice makes the test read `2 == 1`, and the count is the answer), the value comes back rather than the record, the location is the caller's, and recording is not left on. Two deliberate omissions, both in the docstring. Time, because capturing a path and running the sampler cannot happen in one block (#23). And the route, which was implemented and then removed: a captured path is a list of frame names and Base's higher-order functions are in it — three names the reader wrote and seven they did not. Separating them needs `paths` to carry which module each frame came from, which is a change to what `Hit.paths` means. `public`, not exported — `@experimental` remains the only exported name. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
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
1 participant
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.
recordcrashed. Not "returned a wrong number" — signal 11, no Julia backtrace, roughly one fullsuite run in five. It reached CI twice before it was caught, both times a few lines after the
threaded record in
test_spec_profile.jl.The cause is a conjunction, and each half was measured alone
Capturing a call path calls
backtrace(), which unwinds the calling thread. The timing backend'ssampler unwinds those same threads from outside. Two unwinders on one stack.
1.12.7, 150 threaded records per run:
pathstimingThe first stress written for this missed it: 600 records on
-t4, but the work inside themwas single-threaded, so
backtrace()was only ever called from the thread the sampler was leastlikely to be walking. Adding
Threads.@threadsto the recorded block is what turned a guess intoa measurement — and it is why the earlier "
Profile.initafterstop_timeris a use-after-free"hypothesis was dropped rather than shipped: it did not survive its own stress.
What changes
"unsupported".
paths = true, timing = false— the instrument that needs no sampler, andso has no global side effect on
Profile's sampling interval either.record(f; paths = false, timing = true)is the other one.The spec's timing claims pass that keyword explicitly now, and a new testset pins the refusal with
both single-instrument calls as controls: a refusal of the pair is not a refusal of either
half, and without the controls those two readings are the same test.
Nothing else in the package asked for both —
stamptakes the default,assert_cleanasks forneither.
Ordering
PR #22 is red on the same crash, not on its own change. It should go green once this lands.
🤖 Generated with Claude Code