Repository navigation
Conversation
…position decodes one chapter JavaScriptCore finds the source position of a call site in the expression info of its function, which it decodes from the start of a chapter. A chapter was 10,000 words, so the first lookup at each of N call sites in one function cost O(N). The engine of oven-sh/WebKit#995 starts a chapter every 256 words. WEBKIT_VERSION is the preview build of that pull request: the engine that main pins (4d45755fed) plus the two commits. Move it to a commit of oven-sh/WebKit main with that change before this lands. jscInternals.expressionInfoStats() (bun:internal-for-testing) returns the words and the chapter starts of the expression info of the code that calls it. bun-jsc.test.ts checks the line of 400 call sites in a function, a generator and an async function, and that each of the three has chapters of 256 words. bundler_bytecode_portable.test.ts: 17 hashes and 17 sizes move, each output 0.02% to 0.11% larger. A chapter is 16 bytes.
Collaborator
Author
Collaborator
Author
|
Status. Reproduced with The branch has main at e57d89b. The pin is the preview build of oven-sh/WebKit#995 at CI on f3d2005, build 137748: 129 of 188 jobs passed. The two test files that this pull request changes fail on no lane. 28 test files are red, and this change touches none of them:
|
This branch has not been deployed
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.
Problem
bytecode/ExpressionInfo.cpp:901). A chapter is 10,000 words, so N call sites in one function cost O(N) each.bun test --reporter=junitasks for the line of eachdescribe()call: +171,969 instructions each in a file of 2,000. No issue reports it. It is not a regression.Fix
WEBKIT_VERSIONis the preview build of [JSC] ExpressionInfo: start a chapter every 256 words. BytecodeRewriter: keep running sums WebKit#995:main's engine plus that change. Move it to a commit of oven-sh/WebKitmainwith that change before this lands.describe()calls take 13.1 ms, was 23.9 ms.bun-jsc.test.tschecks the lines of 400 call sites in a function, a generator and an async function, and their 256-word chapters.bun bd test test/js/bun/jsc/bun-jsc.test.ts(54 pass, 3 new tests fail on main).bundler_bytecode_portable.test.ts: 17 hashes and sizes re-blessed.Background
WEBKIT_VERSION(scripts/build/deps/webkit.ts) names the oven-sh/WebKit release the build links.autobuild-preview-pr-<n>-<sha>is the build of a pull request there.Downsides
bun build --bytecodeoutput grows by 16 bytes per chapter: +1,672 B (+0.02%) for TypeScript's_tsc.js.src/alone back on main, the new tests fail for the missing helper. The engine's proof is in [JSC] ExpressionInfo: start a chapter every 256 words. BytecodeRewriter: keep running sums WebKit#995.Notes
What is in the diff
scripts/build/deps/webkit.ts: the pin, the preview build of [JSC] ExpressionInfo: start a chapter every 256 words. BytecodeRewriter: keep running sums WebKit#995 at 0f1ec592ee. The old value appears nowhere else in the repo.src/jsc/bindings/JSCTestingHelpers.cpp:jscInternals.expressionInfoStats()forbun:internal-for-testing. It reads the code block of its caller, because a generator or an async function keeps its expression info in the code block of its body, which no function object gives.test/js/bun/jsc/bun-jsc.test.ts: three tests, 120 to 210 ms each on a debug ASan build.test/bundler/bundler_bytecode_portable.test.ts: the snapshot. 17jschashes and 17 sizes move and nojshash. The sizes grow by 48 to 4,488 bytes, 12,120 bytes over 44.0 MB. I ranbun installintest/first: with a staletest/node_modulesthelibraries.jsrow measures other package versions. Onmainthe old snapshot passes with the fresh install (22 pass).The snapshot was written by a release build with JavaScriptCore built from the pull request's head, and it passes on
bun bdwith the preview build that CI made (22 pass with--timeout 300000: two of its tests take 7 to 16 seconds on a debug ASan build on this machine, over the default 5 seconds).Numbers
All from oven-sh/WebKit#995, where the method is. Two release builds of Bun made the same way,
mainwith the engine at 4d45755fed and with the pull request.bun test --reporter=juniton a file of N linesdescribe("d<i>", () => {});, extra instructions perdescribe()over the run without the reporter, and the wall time of the run:The same file with released builds and with Node (
node --test --test-reporter=junit), median of 20 runs:Real test files: of the 3,543 test files in this repo, 11 have 500 or more
describe,testoritcalls.test/js/bun/xml/xml-test-suite.test.tshas the most (2,007). Under the junit reporter it goes from 99.4 ms to 97.8 ms (89.7M to 37.3M instructions over the run without the reporter).A debugger with a breakpoint: a module of N lines
x += <i>;underbun --inspect-wait, with a breakpoint by URL at the middle line.Debugger::pauseIfNeededthen asks for the line and column of each statement that runs. Instructions of the process: 1,590.8M to 155.9M at N = 5,000, 7,474.6M to 556.9M at N = 20,000 (279 ms to 69 ms with the client). A debugger that is only attached asks for no position: 291.5M before and after at N = 20,000. Without a debugger: 91.3M before and after.Binary: text of a release
bungrows by 1,344 bytes (136,784,361 to 136,785,705), the helper included. A run that asks for no position executes the same instructions:bun testof the file of 2,000 without the reporter runs 79.22M before and 79.15M after.Not in this PR
InternalSourceMap::find,src/sourcemap/InternalSourceMap.rs), which walks up to 63 mappings.bun test --aotreads call-site positions from its own table. This does not change it.Self-review
The engine change was reviewed twice in oven-sh/WebKit#995: 45 concerns raised, 41 addressed. For this half the reviews asked for the fresh install before the re-bless, for an async function and a generator in the test, and for the pin of the amended head. The snapshot did not move with that head.
no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/js/bun/jsc/bun-jsc.test.ts, test/bundler/bundler_bytecode_portable.test.ts