Skip to content

fix(symbols): stop the index merge rescanning the whole index per file - #53

Merged
JRedeker merged 2 commits into
mainfrom
work/work-aba1a41d9aa5417f6e51915e
Oct 25, 2026
Merged

JRedeker merged 2 commits into
mainfrom
work/work-aba1a41d9aa5417f6e51915e

Conversation

@JRedeker

@JRedeker JRedeker commented Oct 25, 2026 •

Copy link
Copy Markdown
Contributor

Problem

For each changed file, _index_folder_with_snapshot (src/lgrep/tools/index_folder.py) rebuilt the whole symbol dict and every occurrence bucket to drop that file's old entries. A build of k files therefore cost k × index size. Profiling a cold build of a 500-file repository showed 108 s of 134 s in those comprehensions (122 million dict.get calls), against about 12 s for parsing. The loop is CPU-bound Python, so it holds the GIL and slows every other search in the shared daemon (LGREP-52).

Change

  • _IndexMerge indexes symbols and occurrence buckets by source file, so a removal touches only the buckets that hold that file's rows.
  • A bucket's first removal filters it into a list the merge owns. A second removal splits it once into runs of consecutive same-file rows, and later removals drop only their own runs. Appends go to plain lists, so cold builds never split.
  • Splitting every touched bucket up front, as the first revision did, made one-file refreshes slower. Run allocation on a large heap is mostly garbage-collector time, so only buckets touched twice are split now.
  • The published index is unchanged, including key order, row order, emptied-bucket removal, and @start_byte duplicate-id suffixing. Lists of the loaded index, which the cache may share, are never mutated.
  • The test-file flag is computed once per file instead of once per occurrence.

Evidence

  • tests/test_index_folder_merge.py compares the published index with the previous algorithm over randomized refresh histories. The histories cover modified, deleted, and added files, truncated walks, forced full re-parses, interleaved rows, empty buckets, and colliding symbol ids. They also check that the loaded index is not mutated.
  • Further tests check that, after the one-time filter and split, removals read only the removed files' own entries, at 200 and at 4,000 files, and that the test-file flag is computed once per file.
  • The tests fail under 8 seeded faults: emptied buckets keeping their position, empty buckets surviving, the symbol path check dropped, owned buckets never split, appends writing into the cached list, and three flatten errors.
  • An independent review passed both revisions. It also ran a 50,000-seed fuzz against the reference with full branch coverage.
  • Full suite: 1269 passed, 1 xfailed. ruff check and ruff format --check are clean.

Timing against the 3.7.2 release:

Case 3.7.2 This change
concord cold build, 500 files 81.6 s 15.1 s
concord full re-parse 118.8 s 21.2 s
427 MB index, busiest single file 0.20 s 0.25 s
427 MB index, 10 files 3.52 s 0.87 s
427 MB index, 100 files 34.9 s 1.44 s

The 427 MB rows time the merge step alone, on 981,821 rows. The saved concord index bodies were identical, field for field.

For every changed file, _index_folder_with_snapshot rebuilt the entire
symbol dict and every occurrence bucket to drop that file's old entries,
so a build of k files cost k x index size. A cold build of concord (500
files) spent 108 s of 134 s profiled in those comprehensions. The
CPU-bound loop holds the GIL and slows the shared daemon's searches.

_IndexMerge indexes symbols and occurrence buckets by source file, and
keeps each bucket as runs of consecutive same-file rows, so removing a
file touches only its own entries. The published index is unchanged,
including key and row order, emptied-bucket removal, and duplicate-id
suffixing. The test-file flag is now computed once per file instead of
once per occurrence.

Measured on concord, against the 3.7.2 release under load average 17-24:
cold build 97-99 s -> 18-24 s, full re-parse 136-145 s -> 24.5 s; the
saved index bodies are identical.

Refs: LGREP-52
Splitting every touched bucket into runs on the first removal made a
one-file refresh slower than the old full filter: on a 427 MB index
(981,821 rows), the busiest file's refresh took 0.58 s against 0.21 s.
Most of that was the cyclic garbage collector rescanning the large heap
while about 67,000 run containers were allocated.

A bucket's first removal now filters it into a list the merge owns.
Only a second removal splits it into runs, and appends go to plain lists,
so cold builds never split. Measured on the same index: one file
0.20 -> 0.25 s, 10 files 3.52 -> 0.87 s, 100 files 34.9 -> 1.44 s. On
concord: cold build 81.6 -> 15.1 s, full re-parse 118.8 -> 21.2 s, with
identical saved index bodies.

Refs: LGREP-52
@JRedeker
JRedeker merged commit 4a51c8d into main Oct 25, 2026
4 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.

1 participant