Skip to content

BigInt("0x…"/"0b…"/"0o…"): linear-time string parse - #35899

Open
robobun wants to merge 3 commits into
mainfrom
farm/5ff878ec/bigint-parse-pow2-linear
Open

BigInt("0x…"/"0b…"/"0o…"): linear-time string parse#35899
robobun wants to merge 3 commits into
mainfrom
farm/5ff878ec/bigint-parse-pow2-linear

Conversation

@robobun

@robobun robobun commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator

Depends on oven-sh/WebKit#353.

Problem

BigInt("0x…"), BigInt("0b…"), BigInt("0o…") parse time is O(n²) in the string length. JSBigInt::parseInt routes every radix through a loop that calls multiplyAdd over the full preallocated digit vector once per lengthLimitForBigInt32 characters (n/k groups × O(n) per group). The output direction, toStringBasePowerOfTwo, is already linear.

for (const n of [50_000, 100_000, 200_000]) {
  const s = "f".repeat(n);
  let t = performance.now(); BigInt("0x" + s);
  console.log(n, (performance.now() - t).toFixed(1), "ms");
}
n (hex chars) bun 1.4.0 node v26.3.0 after
50 000 26.8 ms 0.1 ms ~0.2 ms
100 000 125.5 ms 0.3 ms ~0.4 ms
200 000 362 ms 0.4 ms ~0.7 ms

Sibling of the radix-10 quadratic conversions, split out because the pow-2 fix is trivially linear and independent of any divide-and-conquer work.

Fix

oven-sh/WebKit#353 adds a fast path in JSBigInt::parseInt for power-of-two radix that packs each character's ctz(radix) bits directly into the digit vector, carrying across 64-bit word boundaries for octal. No multiplication. Short inputs that fit in int32 fall through to the existing path so they keep returning BigInt32.

This PR

  • Bumps WEBKIT_VERSION (currently pointing at the preview build; will be updated to the merged sha once JSBigInt::parseInt: O(n) fast path for power-of-two radix WebKit#353 lands).
  • Adds test/js/bun/jsc/bigint-parse.test.ts: roundtrip correctness for hex/binary/octal at every alignment boundary (1..40 hex, 1..130 binary, 1..70 octal to cover the 3-bit span across 64-bit words), cross-radix agreement, leading-zero/whitespace handling, SyntaxError on bad chars, and a linearity check at 250 000 hex chars (before: ~570 ms release; after: <10 ms debug+ASAN).

Verification

Local debug-local build (WebKit from source, ASAN):

              before       after
50k hex       1097 ms      1.9 ms
100k hex      4365 ms      3.4 ms
200k hex      17405 ms     5.8 ms

All 7 tests pass in 160 ms.


[decide:webkit] gate passed · iteration 2 · 2 files touched

passes on PR (with fix)
Test-only change.

Debug/ASAN (expected pass):
$ bun bd test 'test/js/bun/jsc/bigint-parse.test.ts'
$ BUN_DEBUG_QUIET_LOGS=1 bun scripts/build.ts --profile=debug --quiet test test/js/bun/jsc/bigint-parse.test.ts
bun test v1.4.0 (f7c2ba01d)

test/js/bun/jsc/bigint-parse.test.ts:
(pass) BigInt string parse, power-of-two radix > hex correctness [38.50ms]
(pass) BigInt string parse, power-of-two radix > binary correctness [42.07ms]
(pass) BigInt string parse, power-of-two radix > octal correctness (3 bits/char, spans digit boundaries) [42.18ms]
(pass) BigInt string parse, power-of-two radix > cross-radix agreement on large values [4.61ms]
(pass) BigInt string parse, power-of-two radix > leading zeros, whitespace, and 16-bit string storage [7.74ms]
(pass) BigInt string parse, power-of-two radix > maxLength boundary (2^20 bits) [16.84ms]
(pass) BigInt string parse, power-of-two radix > invalid characters still throw SyntaxError [8.84ms]
(pass) BigInt string parse, power-of-two radix > parse is linear, not quadratic [25.33ms]

 8 pass
 0 fail
 348 expect() calls
Ran 8 tests across 1 file. [2.29s]
Exit: 0
diff hotspot
scripts/build/deps/webkit.ts         |   2 +-
 test/js/bun/jsc/bigint-parse.test.ts | 114 +++++++++++++++++++++++++++++++++++
 2 files changed, 115 insertions(+), 1 deletion(-)

gate history · 1 passed · 2 rejected · iteration 2

evidence per changed file
file                                  reads  edits  tests
scripts/build/deps/webkit.ts              3      1      0
test/js/bun/jsc/bigint-parse.test.ts      1      4      0

Bumps WebKit to pick up the JSBigInt::parseInt power-of-two fast path
(oven-sh/WebKit#353). Before: O(n^2) because multiplyAdd runs over the
full digit vector for every group of input characters. After: O(n) by
packing bits directly into the digit vector, mirroring
toStringBasePowerOfTwo. 200k-hex-char string: ~360ms -> ~1ms release.

Adds parse correctness coverage for hex/binary/octal at all digit-word
alignment boundaries plus a linearity check at 250k hex chars.
@coderabbitai

coderabbitai Bot commented Jul 26, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 1 minute

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 60561f9c-478b-4361-9884-86b5eff3924a

📥 Commits

Reviewing files that changed from the base of the PR and between 44f6469 and f7c2ba0.

📒 Files selected for processing (2)
  • scripts/build/deps/webkit.ts
  • test/js/bun/jsc/bigint-parse.test.ts

Comment @coderabbitai help to get the list of available commands.

@robobun

robobun commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator Author
Updated 7:13 AM PT - Jul 26th, 2026

@robobun, your commit f7c2ba0 has 2 failures in Build #82305 (All Failures):

  • test/js/bun/s3/s3.test.ts - crash reported on 🐧 3.23 aarch64
  • 📦 Binary size — 12 over 0.50 MB
  • targetthis build canary: main #79916
    sizeΔ
    bun-darwin-aarch6458.13 MB57.58 MB+564.9 KB
    bun-darwin-x6463.48 MB62.95 MB+544.5 KB
    bun-linux-aarch6470.98 MB70.42 MB+576.0 KB
    bun-linux-x6472.47 MB71.95 MB+528.0 KB
    bun-linux-aarch64-musl64.88 MB64.32 MB+576.0 KB
    bun-linux-x64-musl66.98 MB66.45 MB+544.0 KB
    bun-linux-aarch64-android78.47 MB77.97 MB+512.0 KB
    bun-linux-x64-android80.62 MB80.10 MB+529.2 KB
    bun-freebsd-x6483.07 MB82.56 MB+528.0 KB
    bun-freebsd-aarch6484.84 MB84.31 MB+544.0 KB
    bun-windows-x6480.26 MB79.70 MB+572.0 KB
    bun-windows-aarch6470.86 MB70.34 MB+534.5 KB

    Add [skip size check] to the commit message if this increase is intentional.


🧪   To try this PR locally:

bunx bun-pr 35899

That installs a local version of the PR into your bun-35899 executable, so you can run:

bun-35899 --bun

@robobun

robobun commented Jul 26, 2026

Copy link
Copy Markdown
Collaborator Author

Status: diff is green; waiting on oven-sh/WebKit#353 to merge so WEBKIT_VERSION can be swapped from the preview tag to the merged sha.

Build #82305 (192/196 passed):

Local debug-local build (WebKit compiled from source, ASAN):

hex chars before after
50 000 1 097 ms 1.9 ms
100 000 4 365 ms 3.4 ms
200 000 17 405 ms 5.8 ms

USE_SYSTEM_BUN=1 bun test test/js/bun/jsc/bigint-parse.test.ts: perf test fails at 576 ms for 250k hex chars (threshold 250 ms). With the bump: all 8 tests pass in ~270 ms under debug+ASAN.

@robobun
robobun marked this pull request as draft July 26, 2026 08:06
* From https://github.com/oven-sh/WebKit releases.
*/
export const WEBKIT_VERSION = "549170099226f816a4b204ea1d8fa102fb79eefa";
export const WEBKIT_VERSION = "autobuild-preview-pr-353-4b51ec68";

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.

🔴 WEBKIT_VERSION is pointing at autobuild-preview-pr-353-4b51ec68, an ephemeral preview-build release for the still-unmerged oven-sh/WebKit#353. Preview releases are deleted once the upstream PR merges/closes, at which point every fresh prebuilt-mode build (CI and local) will 404 on the WebKit download. Per the description this is a known TODO — flagging so it doesn't slip: bump to the merged commit sha before landing.

Extended reasoning...

What the bug is

scripts/build/deps/webkit.ts:6 sets:

export const WEBKIT_VERSION = "autobuild-preview-pr-353-4b51ec68";

replacing the previous 40-hex commit sha. The autobuild-preview-pr-* tag is the CI preview release that oven-sh/WebKit publishes for open PRs so downstream Bun PRs can test against them before the WebKit change lands. These preview releases are ephemeral — they are deleted (or become stale/unreferenced) once the WebKit PR is merged or closed and the real autobuild-<sha> release is published for the merged commit.

Code path that triggers it

prebuiltUrl() in the same file builds the download URL directly from this constant:

const version = cfg.webkitVersion;
const tag = version.startsWith("autobuild-") ? version : `autobuild-${version}`;
return `https://github.com/oven-sh/WebKit/releases/download/${tag}/${name}.tar.gz`;

so every cfg.webkit === "prebuilt" build — which is the default for CI and for anyone not running a local WebKit checkout — fetches https://github.com/oven-sh/WebKit/releases/download/autobuild-preview-pr-353-4b51ec68/bun-webkit-<os>-<arch><suffix>.tar.gz. prebuiltDestDir() also keys the cache dir on this string, so there is no fallback to a previously-cached sha.

Why nothing else prevents it

There is no guard in the build system that rejects preview tags or falls back to a pinned release; prebuiltUrl() explicitly accommodates the autobuild- prefix so preview tags work during development. The only thing preventing this from breaking builds today is that oven-sh/WebKit#353 is still open and its preview artifacts still exist.

Impact

If this PR merges as-is and oven-sh/WebKit#353 subsequently merges (which it must, since this PR depends on it), the preview release will be cleaned up. From that point every fresh clone / cache-cold CI runner hits a 404 downloading WebKit and the build fails outright — a regression in something that currently works on main. Even before deletion, pinning main to an unmerged, force-pushable PR branch is fragile.

Step-by-step proof

  1. Merge this PR with WEBKIT_VERSION = "autobuild-preview-pr-353-4b51ec68".
  2. JSBigInt::parseInt: O(n) fast path for power-of-two radix WebKit#353 merges; its preview release autobuild-preview-pr-353-4b51ec68 is deleted per the preview-release lifecycle.
  3. A contributor (or CI) with a cold cache runs bun bd. resolveDep for WebKit computes prebuiltUrl(cfg).../releases/download/autobuild-preview-pr-353-4b51ec68/bun-webkit-linux-amd64.tar.gz.
  4. GitHub returns 404; the fetch step fails; the build aborts before compiling anything.
  5. Every subsequent PR's CI is red until someone lands a follow-up bumping WEBKIT_VERSION.

Fix

Before merging, land oven-sh/WebKit#353 first, then update this line to the merged commit's 40-hex sha (matching the autobuild-<sha> release), e.g.:

export const WEBKIT_VERSION = "<merged-commit-sha>";

The PR description already states this intent ("will be updated to the merged sha once oven-sh/WebKit#353 lands"); this comment is the merge-blocking reminder so it can't be forgotten.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Yes, intentional and tracked: PR is in draft until oven-sh/WebKit#353 lands, then this line gets swapped to the merged 40-hex sha. Leaving this thread open as the merge-blocker.

Comment thread test/js/bun/jsc/bigint-parse.test.ts Outdated
Comment thread test/js/bun/jsc/bigint-parse.test.ts Outdated
@robobun
robobun marked this pull request as ready for review July 26, 2026 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants