[JSC] Stack positions: keep the divot of an instruction, and let a provider find the line and column - #730
Closed
robobun wants to merge 3 commits into
Closed
[JSC] Stack positions: keep the divot of an instruction, and let a provider find the line and column#730robobun wants to merge 3 commits into
robobun wants to merge 3 commits into
Conversation
Upstream c76c52f removed ExpressionInfo::m_cachedLineColumns together with the line and column fields. CodeBlock::lineColumnForBytecodeIndex() then decodes the expression info from the start of the chapter on every call. For one frame that is 1,051 instructions where it was 78, and 30 us for a frame late in a function of 1,000 statements. ExpressionInfo::divotForInstPC() keeps the divot of each instruction it was asked for, as lineColumnForInstPC() kept the line and column.
|
Preview build of f44d2ea: |
The calls to makeStack() are on different lines, so the frames of the caller differ between two stacks.
SourceProvider::lineAndColumnForOffset() is a virtual that the two documentLineColumnForOffset functions ask first. The default returns false, and the line start table answers as before. Bun's provider overrides it, so that the first position in a source does not scan the whole source and does not keep one entry per line.
This was referenced Sep 25, 2026
Collaborator
Author
|
Closing: #734 replaces both halves of this PR. It keeps positions in |
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
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.
On hold. The cache half of this PR is now on its own in #732, which is ready. What stays here is the virtual
SourceProvider::lineAndColumnForOffset(). It waits for a decision on oven-sh/bun#43955 (where the line index should live). If the index goes into the fork, this PR closes.Part of oven-sh/bun#43882.
Problem
7b485a76e9, each frame of a stack trace costs more. One call ofCodeBlock::lineColumnForBytecodeIndex()is 1,051 instructions in a release build of Bun. It was 78.c76c52f5b1. It removedExpressionInfo::m_cachedLineColumnstogether with the line and column fields, so every lookup runsExpressionInfo::entryForInstPC(), which decodes from the start of the chapter.LineStartTable, one entry per line, from a scan of the whole source. For a 10 MB module that is 6.8 ms and 6 MB of RSS at the first read oferror.stack.Fix
ExpressionInfo::divotForInstPC()keeps the divot of each instruction it was asked for, in a hash map, aslineColumnForInstPC()kept the line and column.CodeBlock::lineColumnForBytecodeIndex()asks for the divot through it.SourceProvider::lineAndColumnForOffset()is a virtual that the twodocumentLineColumnForOffsetfunctions ask first. The default returns false, and the table answers as before. Bun's provider overrides it in Find the line of a stack frame without a table of every line bun#43955 (draft).USE(BUN_JSC_ADDITIONS).JSTests/stress/stack-position-is-the-same-on-every-read.js, and the measurements below.Measured on Linux x64 with release builds of Bun: main (WebKit
35e8970dfd), the upgrade (74650443cb), and the upgrade with the cache commit9f0ea1adf1of this PR. Instruction counts come from gdb (stepifrom the entry of the function to its return). Times are medians of interleaved runs.error.stack, 11 framesnew Error().stack, call depth 10error.stack, frame late in module code of 100 statementsBackground
ExpressionInfomaps an instruction to the source range of its expression. It is a compressed stream with a chapter every 10,000 words, and a lookup decodes from the start of a chapter.c76c52f5b1the line and the column come from the source provider, which derives them from the divot.Downsides
ExpressionInfo, and a hash map slot of 8 bytes for each instruction that a stack trace, the sampling profiler or the debugger asked for. The removed cache had the same pointer and slots of 12 bytes.