Skip to content

install: read a .npmrc in linear time with hash tables past 32 keys - #46932

Open
robobun wants to merge 5 commits into
mainfrom
robobun/d403cd65/ini-linear-keys
Open

robobun wants to merge 5 commits into
mainfrom
robobun/d403cd65/ini-linear-keys

Conversation

@robobun

@robobun robobun commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

  • Reading a .npmrc takes time quadratic in its distinct keys. bun pm bin beside 32,000 lines of key<i>=value uses 1,793 ms of CPU.
  • Three scans cause it: Parser::parse (src/ini/lib.rs:371-396), resolve_url_auth (:1289) and UrlAuth::find_entry (src/install/npm.rs:621).
  • No user reported this. A code read for toml: index a table's keys once it outgrows a linear scan #37449 found it, and a real file has tens of keys.

Fix

  • parse looks a key up once per line. KeyIndex scans an object of up to 32 properties and keeps a hash table for a larger one.
  • Collapsed and UrlAuthList do the same for credential lines and credential keys.
  • open_section replaces E::Object::get_or_put_object, which is deleted with its last caller, a bun init branch that cannot run.
  • Verified: 13 CPU-time rows in test/js/bun/ini/ini.test.ts and test/cli/install/npmrc.test.ts fail on main. Output matches main on 9,000 random files. Self-reviewed: 34 concerns raised, 29 addressed (see Notes).

Background

  • The parser builds an E::Object, the AST node of an object literal. Its keys are a list, so a lookup is a scan.
  • A [section] can reopen an object that the JSON parser built, so the table reads the property list.
  • Considered an index in bun_ast (one consumer) and an always-on map (2 allocations per file). A 5-line file makes the same allocator calls as on main.

Downsides

  • Opening the first of more than 32 sections again costs about 12 ns more. A hash replaces a scan that stopped at once.
  • A table built for 34 to 38 section headers is hardly used (+5,543 instructions at 34).
  • .text grows by 12,800 bytes (release, linux x64).
Notes

All numbers: release builds of main 179664e and this branch, linux x64, unless a line says debug.

What this is. It removes accidental quadratic work, for a file that is much larger than a real one. It is not a fix for hostile input: a very large .npmrc still costs memory in proportion to its size, as on main. #37449 removes the same scan from the TOML parser with the same shape: a scan up to a limit, a hash table past it. The limit is 32 here (as DUP_LINEAR_MAX in src/parsers/json_stage2.rs) and 64 there, so one decision can settle both.

bun pm bin beside a .npmrc of N lines, with HOME set to an empty directory: CPU ms of the process at N = 4,000 / 8,000 / 16,000 / 32,000 (best of 3, interleaved)

.npmrc lines main this PR
key<i>=value 27 / 101 / 427 / 1,793 3.8 / 5.7 / 7.9 / 12
key<i>[]=value 45 / 197 / 851 / 3,805 3.7 / 5.6 / 8.4 / 16
[s<i>] 14 / 59 / 254 / 1,054 3.0 / 4.2 / 6.9 / 11
[a.b<i>] 15 / 72 / 311 / 1,297 2.9 / 4.7 / 6.8 / 14
N keys in one section 24 / 99 / 411 / 1,828 3.1 / 4.3 / 6.1 / 11
a JSON value, then [foo] adds keys 18 / 69 / 322 / 1,337 3.0 / 4.2 / 5.9 / 11
two sections, a header before each key 5.6 / 15 / 60 / 286 2.5 / 3.6 / 4.8 / 7.8
sections of 32 keys 2.6 / 4.0 / 5.9 / 12 2.8 / 3.6 / 5.0 / 9.8
sections of 34 keys 3.4 / 5.0 / 7.8 / 14 3.6 / 4.8 / 6.9 / 13
parents of 34 child sections 4.2 / 6.2 / 17 / 47 4.0 / 5.7 / 8.8 / 16
one repeated key 2.6 / 2.8 / 3.4 / 4.7 2.3 / 2.5 / 3.6 / 4.4
//host<i>/:_authToken= 45 / 197 / 743 / 3,339 5.4 / 9.7 / 17 / 33
//host<i>/:username= 58 / 240 / 926 / 4,113 4.6 / 7.3 / 12 / 23
//host<i>/:_auth= (empty) 60 / 264 / 950 / 4,388 4.5 / 7.7 / 13 / 23
@s<i>:registry= 28 / 112 / 468 / 2,023 6.1 / 9.5 / 19 / 37
@s<i>:registry= and an empty _auth for another host 37 / 164 / 732 / 2,789 6.1 / 9.6 / 17 / 32
@s<i>:registry= and _authToken for its host 31 / 136 / 629 / 2,419 6.4 / 9.6 / 19 / 37
@s<i>:registry= and _authToken for another host 41 / 180 / 820 / 3,210 6.2 / 10 / 19 / 35
the same, registry under 4 path segments 72 / 310 / 1,462 / 6,031 6.3 / 12 / 21 / 43
the same, registry under 6 path segments 92 / 380 / 1,819 / 7,277 7.1 / 12 / 22 / 40

Without a .npmrc the command uses about 2 ms. Nothing that ships reads a key under a [section] of a .npmrc: parse_npmrc_into reads root keys only. The section rows are CPU that main spends to build output that only the test hook reads.

The sizes of real files (best of 15, interleaved, noise about 0.2 ms)

distinct keys 5 20 100 200 400 700 1,000 2,000
main, ms 1.62 1.45 1.45 1.50 1.94 2.26 3.39 7.41
this PR, ms 1.41 1.42 1.51 1.49 1.60 1.63 1.80 2.29

The saving reaches 1 ms between 700 and 1,000 distinct keys.

Small files (each count is exact and repeats)

  • Parser::parse allocator calls (mi_heap_malloc / mi_malloc / mi_free), 5-line .npmrc: 20 / 2 / 10 → 20 / 2 / 10. 20 keys: 80 / 4 / 40 → 80 / 4 / 40. 8 lines with headers and key[]: 30 / 5 / 13 → 30 / 5 / 13 (gdb breakpoint counters).
  • load_npmrc_config allocator calls, 5-line .npmrc with one token: 20 / 18 / 16 → 20 / 18 / 16. 8 lines with 3 credential lines: 32 / 26 / 26 → 32 / 26 / 26.
  • Options::load allocator calls (mi_malloc / mi_zalloc / mi_free), 5-line .npmrc with one token: 21 / 1 / 12 → 21 / 1 / 12. 8 lines with 3 credential lines: 36 / 2 / 20 → 36 / 2 / 20.
  • Parser::parse instructions: 5 lines 32,817 → 32,392. 20 keys 64,419 → 55,157. 32 keys 106,557 → 84,233. 34 keys 115,259 → 95,046. 40 keys 141,270 → 102,882. 80 keys 386,913 → 170,678 (gdb stepi, without the vDSO clock read). For each added key from 40 to 80 keys: 6,141 → 1,695. On this branch from 80 to 160 keys: 1,683, so the cost of a key is constant.
  • load_npmrc_config instructions, 5-line .npmrc with one token: 55,395 → 55,013, of which outside Parser::parse 22,578 → 22,621. 8 lines with 3 credential lines: 65,389 → 64,525.
  • Options::load instructions, 5-line .npmrc with one token: 18,278 → 18,331. 8 lines with 3 credential lines: 24,872 → 24,914.
  • Calls into the table arm of KeyIndex for each parse: 5 lines 0, 20 keys 0, 32 keys 0, 33 keys 0, 34 keys 1 (gdb breakpoint hit count).
  • Size: total .text +12,800 bytes, bun_ini symbols +11,857 bytes, UrlAuthList::find_entry +763 bytes (size and nm -S). E::Object::get_or_put_object (972 bytes) is gone. Options grows by one StringHashMap<u32>, once per process.
  • Peak RSS at 64,000 lines: key<i>=value 36 → 39 MB, _authToken lines 66 → 70 MB.

Slower than main

  • 40 sections, then [s0] again on each of 256,000 lines: 19 → 22 ms of CPU. With [s39] on each line: 35 → 24 ms.
  • Parser::parse instructions for N [sec<i>] headers: 33 headers 76,094 → 76,047. 34 headers 78,392 → 83,935. 35 headers 80,739 → 85,140. The gap closes by about 1,140 for each header after that.

Tests

  • CPU rows: each spawns bun pm bin beside a generated .npmrc and checks the CPU time of the child (proc.resourceUsage()) against a limit, as test/cli/format/format.test.ts does. Release: 64,000 lines, limit 1 s. These rows take 0.02 to 0.08 s on this branch and 4.9 to 29 s on main. Debug: 16,000 or 8,000 lines, limit 4 s. They take 0.3 to 1.1 s on this branch and 8 to 53 s on main. Release with ASAN and assertions (bun run build:asan, as on the x64-asan lane): 64,000 lines, limit 4 s. They take 0.2 to 0.7 s on this branch and 22 to 87 s on main.
  • The rows set no timeout of their own. With the 90 s that CI sets, a row of main fails on the CPU check. With the default of 5 s in a local run, it fails on that timeout, and its child runs on until it ends: the test runner does not kill the children of a concurrent test. A row of this branch takes up to 3.3 s of wall-clock time in a debug build. Most of that is the test process, which builds the file.
  • No failure in 96 runs of those rows with 24 test processes at once and the default timeout (release). The slowest row took 1.7 s of wall-clock time.
  • Each part of the change has a row that fails without it, checked with one build per part. Key lookup back on a scan: 5 parser rows and 6 credential rows. Header walk back on a scan: the 2 section rows. No credential table: 6 rows. No set of seen keys: 2 rows, release only. No table in UrlAuthList: the row with registries under a path. One table for all objects, or one table at a time: the behaviour rows crash, or the row with two sections fails. The set of registry keys has no dependable row: its scan costs 3 ms at 4,000 lines.
  • Behaviour rows in ini.test.ts: 15 cases, each with 0, 31, 32, 33 and 300 other keys in the object. 24 random files read with and without 33 more keys in each object. Two keys with the same hash. 4,000 keys and 4,000 sections. They check values and key order, and they pass on main too.
  • npmrc.test.ts: 22 cases past the limits (last line wins across files and spellings, an empty value clears an earlier one, the diagnostic for an empty _auth by each spelling of a key, a token among 4,000 credential lines, what a [section] header does to the options).
  • Differential runs against main: iniInternals.parse on 6,600 random files, resolved credentials of 1,750 random files, the stderr of bun pm bin for 400 random files, and the Authorization header of 2,532 requests from bun install for 390 random files. No difference.
  • Also ran redacted-config-logs, config-precedence, public-hoist-pattern, bun-pm, bun-info, bun-publish and bun-install-pathname-trailing-slash under test/cli/install/, test/cli/init/init.test.ts and test/js/bun/toml/toml.test.ts.
  • Not run here: Windows and macOS.

Deleted

Self-review: not taken

  • A timing helper in test/harness.ts: 11 test files read the CPU time of a child inline.
  • A segment list in place of E::Rope: the functions that split a header have no tests yet.
  • An ordered map for the credential list from the parser to Options: it costs about 40 ns more for each tarball URL of a small file.
  • A table that KeyIndex builds on the second lookup: more state to save one build.
  • A lint for the scan functions: it cannot see the credential scans.

Not in this change


no test proof · iteration 0 · platform-specific test(s) that do not run on this machine, deferring to CI, which covers all platforms: test/cli/install/npmrc.test.ts

`Parser::parse` scanned the object for every line: two scans for
`key = value`, four for `key[] = value`, and one for each segment of a
`[section]` header. A file of N distinct keys took N^2 key compares.

`parse` now looks the key up once per line. `KeyIndex` scans an object
of up to 32 properties, as before. For a larger object it keeps a hash
table of the positions of the keys, with the hash of each key. It reads
the table from the object's own property list, so it also has the keys
of a JSON value that a later `[section]` adds to. A debug build checks
the table against a scan for an object of up to 512 properties.

`open_section` walks the segments of a header with its lookups through
`KeyIndex`. It applies the rules of `E::Object::get_or_put_object`,
which the parser does not call any more.
`resolve_url_auth` scanned every credential line for each line: up to
four `rfind` passes to collapse a key, one pass over `install.url_auth`
to skip a key that has its entry, and one pass over the registries for
each empty `_auth` line.

`Collapsed` keeps the last line of each `(key, option)` in a hash table
when there are more than 32 credential lines, with a set of the keys of
the lines so far. More than 32 registries get a set of their keys. Up
to 32, each is scanned as before.
`UrlAuth::find_entry` scanned every credential key for each key of the
walk of a URL. `Options::load` calls it for each registry, so many
registries with many credential keys took their product in compares.

`UrlAuthList` holds the keys. With more than 32 of them, it finds a key
through a hash table of the first entry of each key. Up to 32, it scans
them as before.
The INI parser was the last caller of `E::Object::get_or_put_object`
that can run. The other caller, `Template::write_to_package_json` in
`bun init`, cannot run: `exec` returns for every React template before
it reaches that call.

Delete the walker with `E::SetError` and `Rope::next_ref`. Delete
`write_to_package_json` with `Template::is_react` and
`Template::scripts`, which only it used.
@coderabbitai

coderabbitai Bot commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review.

Recent review info
Run configuration
Commits

Reviewing files that changed from the base of the PR and between 179664e and e1b16db.

Files selected for processing: 7
  • src/ast/e.rs
  • src/ini/lib.rs
  • src/install/PackageManager/PackageManagerOptions.rs
  • src/install/npm.rs
  • src/runtime/cli/init_command.rs
  • test/cli/install/npmrc.test.ts
  • test/js/bun/ini/ini.test.ts
Files with no reviewable changes: 1
  • src/runtime/cli/init_command.rs

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 6 remain after this review.

Walkthrough

The INI parser now indexes object properties and traverses section headers. npm configuration credential resolution and URL-auth lookup use indexed lookups for larger inputs. React initialization no longer uses helpers that add template names and scripts to package.json.

Changes

INI and npm configuration lookup

Layer / File(s) Summary
Indexed INI property and section parsing
src/ast/e.rs, src/ini/lib.rs, test/js/bun/ini/ini.test.ts, test/cli/install/npmrc.test.ts
The parser uses an index for property lookups in larger objects and updates existing properties in place. Section headers traverse or create nested objects. Tests cover duplicate keys, arrays, nested and reopened sections, key counts around the index threshold, and large inputs.
Credential resolution and URL-auth lookup
src/ini/lib.rs, src/install/npm.rs, src/install/PackageManager/PackageManagerOptions.rs, test/cli/install/npmrc.test.ts, test/js/bun/ini/ini.test.ts
Credential resolution collapses duplicate (registry URL, option) entries to the last value and avoids repeated registry-key work. UrlAuthList indexes entries for larger lists and is used by package manager lookups. Tests cover credential precedence, registry key matching, diagnostics, and performance.

React template initialization

Layer / File(s) Summary
React template package.json handling
src/runtime/cli/init_command.rs
The React-specific helper path that added template names and scripts to package.json was removed, along with template script definitions.

Suggested reviewers: jarred-sumner, dylan-conway

Priority: Normal

Merge Risk: Minimal · up to e1b16

The change makes .npmrc and INI parsing scale linearly on large files while preserving existing results. The removed bun init code was unreachable, so React template output is unchanged. No merge-blocking risk was found.

Pre-merge checks: Passed 4
Check name Status Explanation
Title check Passed The title names the main change: a .npmrc is read in linear time, using hash tables once a collection passes 32 keys. The changeset matches this. It adds a per-object key index in the INI parser and a…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.
Description check Passed The description is complete and directly addresses the PR purpose, implementation, verification results, performance impact, limitations, and platform coverage. It does not use the template headings "…
Full details: Title check

Explanation

The title names the main change: a .npmrc is read in linear time, using hash tables once a collection passes 32 keys. The changeset matches this. It adds a per-object key index in the INI parser and a hash-indexed UrlAuthList for credential lookups, and it removes the get_or_put_object path that the title does not mention.

Full details: Description check

Explanation

The description is complete and directly addresses the PR purpose, implementation, verification results, performance impact, limitations, and platform coverage. It does not use the template headings "What does this PR do?" and "How did you verify your code works?", but the required information is present in the Problem, Fix, Tests, and related sections.

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

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

@robobun

robobun commented Oct 25, 2026 •

Copy link
Copy Markdown
Collaborator Author
Updated 5:48 PM PT - Oct 25th, 2026

✅ @robobun, your commit e1b16db906e475026f5bb9bbf1bfd1e8c0120bd7 passed in Build #132831! 🎉


🧪   To try this PR locally:

bunx bun-pr 46932

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

bun-46932 --bun

@robobun

robobun commented Oct 25, 2026

Copy link
Copy Markdown
Collaborator Author

Status: reproduced, and the fix is in this PR (#46932).

How I reproduced it, on release builds of main 179664e (linux x64):

  1. Write a .npmrc of N lines key<i>=value beside a package.json that holds {"name":"x","version":"1.0.0"}.
  2. Set HOME to an empty directory, so that no other .npmrc is read.
  3. Run bun pm bin in that directory and read the CPU time of the process.
N 4,000 8,000 16,000 32,000
main, ms 27 101 427 1,793
this PR, ms 3.8 5.7 7.9 12

@claude claude Bot left a comment

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.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline nit, I traced the rewritten key/value path in Parser::parse against the old get/put sequence for every combination of key[] vs plain key and no-existing / existing-scalar / existing-array value, and open_section against the deleted get_or_put_object (array-last-element descent, empty array, type clobber, created-object loc and flags) — the results and property order match. The deleted bun init branch is unreachable: the React templates return at init_command.rs:624 before the is_react() call. The >32 dedupe via seen in resolve_url_auth yields the same url_auth list as the scan because credentials_of_key depends only on the collapsed map and the key.

Extended reasoning...

The PR rewrites key lookup in the INI parser (src/ini/lib.rs) and .npmrc credential collapse and UrlAuth lookup (src/install/npm.rs, PackageManagerOptions.rs), adding hash-table paths past 32 entries, and deletes get_or_put_object plus a dead bun init branch. It touches registry credential resolution, which is auth-sensitive, and the fast/slow split means two code paths must agree exactly. I found no behavior difference between the scan and table paths or against the base branch, and no new unsafe code, but the parser rewrite is large enough and credential-adjacent enough that a human look is still worthwhile rather than an approval.

Comment thread test/js/bun/ini/ini.test.ts Resolved
The rows pass under the default timeout. A row takes at most 3.3 s in a
debug build with ASAN, and about 0.1 s in a release build.

@claude claude Bot left a comment

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.

Nothing blocking. The comments below are optional suggestions. There is no need to push a fix for them before merging.

Beyond the inline finding, I also diffed the new open_section/per-line key path in src/ini/lib.rs against the deleted get_or_put_object and Object::put (first-match replacement, array promotion, array-last-element descent, __proto__ flags via append_property) and found them equivalent; the seen map in resolve_url_auth and the first-entry index in UrlAuthList match the scan paths they replace; and the deleted bun init branch is unreachable because React templates return at src/runtime/cli/init_command.rs:622-625.

Extended reasoning...

The change rewrites key lookup in the INI parser and .npmrc credential resolution to use hash tables past 32 entries, touching credential handling (url_auth lookup by registry URL) but keeping the same first/last-entry semantics as the scans it replaces. The refactor-equivalence checks above were done by reading the old and new code side by side; the only reported finding concerns CPU-time bounds in the new tests on slow lanes.

Comment thread test/js/bun/ini/ini.test.ts Resolved

This branch has not been deployed

No deployments
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.

1 participant