Skip to content

perf(index): file interning — qualified drops 242k per-entry Urls for 20-byte SymbolLocs#208

Merged
Hessesian merged 3 commits into
refactor/unified-resolutionfrom
perf/f3-file-interning
Jul 8, 2026
Merged

perf(index): file interning — qualified drops 242k per-entry Urls for 20-byte SymbolLocs#208
Hessesian merged 3 commits into
refactor/unified-resolutionfrom
perf/f3-file-interning

Conversation

@Hessesian

Copy link
Copy Markdown
Owner

Summary

F3 steps 1–2 of the memory plan (design: docs/superpowers/plans/2026-07-05-post-fable-roadmap-refinements.md §1): introduce FileId(u32) + FileTable (one interned Arc<Url> per file, race-safe double-checked intern) and migrate qualified from DashMap<String, Location> to DashMap<String, SymbolLoc { file: FileId, range }> — 20 bytes inline instead of 104 bytes + a heap Url per entry.

  • Boundary rule: a tower_lsp::Location is reconstructed from a SymbolLoc in exactly one place (FileTable::location, used by the sole surfacing reader resolve_via_imports); other readers are key-existence/removal and never materialize a Url.
  • No cache bump: qualified is never serialized — it's rebuilt at apply time from each entry's own URI + qualified_keys; the existing v29 cache loads unchanged.
  • FileTable is append-only by invariant, not convention: reset_index_state retains library entries, so their SymbolLocs must stay resolvable; FileIds are stable and never reused, growth is bounded by distinct files per session.
  • Probe updated to measure the new shape (qualified: SymbolLocs + file_table rows).

Measured (probe, same 117 MB / 242,467-entry corpus)

before after
qualified value storage 36.04 MB (per-entry Urls) 4.62 MB SymbolLocs + 5.36 MB shared FileTable
RSS apply peak 408 MB 333 MB

Cumulative since the plan started: warm-load peak 487 → 333 MB. Next targets: definitions (33.6 MB Vec buffers + 23.1 MB Urls), then subtypes/packages.

1440 tests green, clippy -D warnings clean, per commit. Task-scoped review: Approved — intern atomicity, consumer census (grep + cargo check --tests), URL-identity semantics, and probe honesty all independently verified; no Critical/Important findings.

🤖 Generated with Claude Code

https://claude.ai/code/session_01EJsyQ1UgF8uJYR4HgpxEBB

Hessesian and others added 2 commits July 6, 2026 14:46
Add `FileId(u32)`, `SymbolLoc { file, range }`, and an append-only
`FileTable` (RwLock<Vec<Arc<Url>>> + DashMap<uri, FileId>) in types.rs,
plus a `file_table` field on the Indexer. Every file that enters `files`
or `jar_files` is interned via `register_file_uri` at insert time.

The table is populated but not yet consumed: `SymbolLoc`, `FileTable::url`
and `FileTable::location` are gated with `#[allow(dead_code)]` until the
`qualified` migration (next commit) reads through them. Suite green.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EJsyQ1UgF8uJYR4HgpxEBB
`Indexer::qualified` becomes `DashMap<String, SymbolLoc>`: each value is an
interned FileId + Range (20 B inline, no heap) instead of a full Location
whose parsed Url string was duplicated across every symbol in a file. All
five writers (apply flush/fresh/dedup paths, sources-JAR + compiled-JAR
enrichment) intern the URI at insert time via `Indexer::intern_location`
(idempotent: reuses the FileId registered at file-insert). The one reader
that surfaces a Location — `resolve_via_imports` — rebuilds it at the return
boundary through `FileTable::location`; `reset_index_state`'s library-retain
compares the file's interned URI instead. `find_definition_qualified`'s
`contains_key` reader is unchanged.

Cache format is untouched: index.bin serializes per-file `qualified_keys`
(key + Range) and rebuilds the map from each entry's own URI at apply time,
so no CACHE_VERSION bump (verified: the probe loads the existing v29 cache
unchanged). Probe accounting updated: the old "qualified: Location URIs"
row (heap URIs) splits into "qualified: SymbolLocs" (inline) + a new
"file_table: interned URIs" row (one Arc<Url> per file).

Probe (117 MB corpus, 12,621 files / 242,467 qualified entries):
  qualified: Location URIs  36.04 MB  ->  qualified: SymbolLocs   4.62 MB
                                          file_table: interned URIs 5.36 MB
  accounted total          269.40 MB  ->                        243.97 MB
  RSS apply peak           409.4  MB  ->                        333.1  MB

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EJsyQ1UgF8uJYR4HgpxEBB

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

This PR reduces index memory usage by interning file URIs in a new FileTable and replacing per-entry Location values in the qualified index with compact SymbolLoc { file: FileId, range }, reconstituting full Locations only at the LSP boundary.

Changes:

  • Added FileId, SymbolLoc, and an append-only FileTable to intern Urls once per file.
  • Migrated Indexer::qualified from DashMap<String, Location> to DashMap<String, SymbolLoc> and reconstructed Location only in resolve_via_imports.
  • Updated apply/JAR pipelines, memory probe accounting, and tests to use the interned representation.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
src/types.rs Introduces FileId, SymbolLoc, and FileTable (interning + boundary reconstruction).
src/indexer.rs Updates Indexer to store qualified: SymbolLoc plus a file_table, and adjusts retention logic.
src/indexer/apply.rs Adds register_file_uri and intern_location, and migrates qualified inserts to SymbolLoc.
src/indexer/jar.rs Registers file URIs and migrates qualified inserts to interned SymbolLoc.
src/resolver/resolve.rs Reconstitutes Location from SymbolLoc at the qualified-resolution return boundary.
src/indexer/memory_probe_tests.rs Updates probe to size qualified as SymbolLoc and accounts for file_table interned URIs.
src/indexer/jar_tests.rs Updates JAR tests to resolve qualified entries via file_table.location.
src/indexer_tests.rs Updates tests inserting into qualified to use intern_location.

Comment thread src/types.rs
Comment thread src/resolver/resolve.rs
…ified→file_table invariant

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EJsyQ1UgF8uJYR4HgpxEBB
@Hessesian Hessesian merged commit 01b2654 into refactor/unified-resolution Jul 8, 2026
3 of 4 checks passed
Hessesian added a commit that referenced this pull request Jul 8, 2026
…ntry Urls (#209)

* perf(index): definitions stores SymbolLoc — drop 156k per-entry Urls

`definitions: DashMap<String, Vec<Location>>` held 155,909 `Location`s, each
carrying its own heap `Url` (104 B inline + the URI string) even though the same
file's URI already lives once in `file_table` (interned by PR #208 for
`qualified`). Store `Vec<SymbolLoc>` (4-byte `FileId` + range) instead.

Writers (apply.rs, jar.rs) intern each `Location` via the existing
`intern_location` helper at merge time; the transient `FileContributions` /
`LibraryBatch` builders keep building plain `Location`s and intern at flush,
mirroring `qualified`. Readers convert at the LSP return boundary via
`FileTable::location` (or `FileTable::url` where only the URI string is needed
for a further lookup). `remove_stale_for_uri` compares a single same-source
`FileId` (the removed file, re-interned idempotently), not a cross-source URI
string — no normalization divergence introduced.

Not serialized: `definitions` is rebuilt at apply from `file_data.symbols`, so
CACHE_VERSION is unchanged (stays 29).

Probe (real 117 MB corpus): definitions 58.9 MB → 9.6 MB (−49.3 MB); accounted
total 243.6 → 194.3 MB; warm-apply peak 331 → 278 MB. Full suite green
(1440 + 1 ignored), clippy -D warnings, fmt.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EJsyQ1UgF8uJYR4HgpxEBB

* perf(index): subtypes stores SymbolLoc — same interning as definitions

`subtypes: DashMap<String, Vec<Location>>` (the goToImplementation reverse index)
has a tiny reader surface — the single `subtypes_of` boundary returns
`Vec<Location>`; every feature (implementation, fill_when, traits) goes through
it. Apply the same `SymbolLoc` migration as `definitions`: writers intern at
merge (apply.rs, jar.rs), `subtypes_of` reconstitutes at the LSP boundary,
`reset_index_state` / `remove_stale_for_uri` filter by interned `FileId` /
`file_table.url`. Not serialized (rebuilt from `supers` at apply) — CACHE_VERSION
unchanged.

Probe: subtypes 5.2 MB → 0.6 MB (−4.6 MB); accounted total 194.3 → 189.6 MB.
Full suite green (1440 + 1 ignored), clippy -D warnings, fmt.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EJsyQ1UgF8uJYR4HgpxEBB

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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