Skip to content

perf(ts_test): share npm binding sets by content and find hidden runfiles paths in one sorted pass - #386

Merged
mikn merged 2 commits into
mainfrom
aronstrandberg/analysis-perf
Nov 5, 2026
Merged

mikn merged 2 commits into
mainfrom
aronstrandberg/analysis-perf

Conversation

@aronstrandberg

@aronstrandberg aronstrandberg commented Nov 4, 2026 •

Copy link
Copy Markdown
Member

In the monorepo, the bazel-frontend-unit and bazel-workers-tests lanes usually start with a cold Bazel server, and on a PR with no web changes the whole lane is analysis. About 70% of the profiled analysis time is this ruleset's Starlark, mostly _ts_test_impl. This PR removes the biggest pieces of repeated work there without changing any action.

Binding sets shared by content. runtime_npm_contexts interned npm_binding_sets by "<owner> <lookup directory>", so every directory got its own copy even when it resolved to the same map. //web/shared:shared_test wrote 1,093 sets, of which 6 were distinct: 17.3 MB of its 19.4 MB launcher config. Each config went through json.encode and ctx.actions.write (which compresses large contents) during analysis. Sets are now also deduplicated by their sorted items. Across the 118 test programs in the two lanes, the written files shrink from 4.4 GB to 3.1 GB.

Hidden runfiles paths in one sorted pass. _runfiles_paths and _declare_native_view checked every runfiles path against all of its ancestor prefixes, building each prefix with "/".join(parts[:depth]), which is quadratic in depth per path. _beneath_another sorts the paths by components, so each subtree directly follows its root, and marks a path hidden when it starts with the last root seen. The visible set and its insertion order are unchanged.

One runfiles flattening per test. ts_test called runfiles_scope_paths twice on the same runfiles, once in _ancestor_scope_links and once for the module check. Each call flattened and filtered the whole runfiles tree. It now computes runfiles_visible_paths once and passes it to both. It recomputes only when scopes were staged in between, because staging changes the runfiles.

Cheaper path strings. runfiles_scope_paths walks each demanded path up with rpartition and stops at the first ancestor already recorded. rlocation_path reads short_path once and returns without building and stripping external/. runtime_scope_destinations cuts the trailing components with rpartition instead of splitting and re-joining the whole runtime path.

Equivalence. I ran bazel aquery --include_file_write_contents --include_commandline --include_artifacts over deps() of all 118 test programs in bazel-frontend-unit and bazel-workers-tests, with this repo as --override_module at the monorepo's pin (5d5fba8) with and without the change. I compared the dumps per action: 35,500 actions. With every bindings_set index resolved to its map, every action, command line and written file is identical. Without the binding-set change the raw dumps are byte-identical.

Measurements. I measured cold analysis (bazel build --nobuild, after bazel shutdown) of those 118 test programs on a shared 16-core workstation. I alternated pinned and patched runs to spread the noise, and recorded the Bazel server's total CPU from /proc. The patched run was faster in all six pairs.

Server CPU (median of 6) Wall (median of 6)
5d5fba8 895 s 75.9 s
5d5fba8 + this change 815 s (−9%) 67.7 s (−11%)

Profile by profile, Starlark CPU went from about 226 s to 189 s. write and json.encode fell from 39 s to 18 s, and _runfiles_paths from 32 s to 7 s cumulative.

I haven't run this repo's own test suite locally; the workstation is too loaded for the nested-Bazel tests. CI here should cover the analysis tests that exercise runfiles_scope_paths.

//tests/compile_layout/nested_scope:scoped_test_npm_contexts_test pinned the old npm_binding_sets line, which listed the same zod map three times. It now expects [{zod}, {}], with every context's bindings_set at 0, the same resolved bindings.

@aronstrandberg
aronstrandberg marked this pull request as ready for review November 4, 2026 09:29
@aronstrandberg
aronstrandberg requested a review from mikn as a code owner November 4, 2026 09:29
@mikn
mikn merged commit 5dd8673 into main Nov 5, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants