Skip to content

Commit 535d381

Browse files
committed
Lower memory use in the cross-file reference index
1 parent 4f15fec commit 535d381

9 files changed

Lines changed: 141 additions & 155 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
4242
- **Continuous progress reporting.** The indexing progress bar now advances file by file with live counts (e.g. "Scanning vendor packages (3201/8544 files)") instead of jumping between a few fixed milestones. This covers single-project, monorepo, and non-Composer workspaces. Go to Implementation, Find References, and Type Hierarchy show the same live progress while they scan, including when one of them triggers the first full workspace index.
4343
- **Property hover now shows effective types as a `var` detail line.** Property hovers now mirror method hovers by displaying the resolved/effective property type above the PHP snippet as `**var**`, while the snippet itself shows only the native PHP property declaration. This keeps docblock-inferred, virtual, and schema-derived property types out of the generated signature block. Contributed by @calebdw.
4444
- **Updated the bundled mago toolchain to 1.43.0.** The parser, docblock parser, formatter, and supporting crates are refreshed to the latest upstream release. Contributed by @enwi in https://github.com/PHPantom-dev/phpantom_lsp/pull/234.
45+
- **Lower memory use in the cross-file reference index.** The index backing Find References and the reference-count inlay hints now keeps only the distinct files and counts each symbol actually needs, instead of one entry per matching location plus never-read position data. On large projects this removes millions of short-lived allocations and shrinks the index to a fraction of its previous size, with no change to Find References or inlay hint results.
4546

4647
### Removed
4748

docs/todo.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ within the same impact tier.
2626
| # | Item | Impact | Effort |
2727
| --- | --------------------------------------------------------------------------------------------------------------------- | ---------- | ------ |
2828
| P36 | [Diagnostic-path call sites re-merge inheritance per call](todo/performance.md#p36-diagnostic-path-call-sites-re-merge-inheritance-per-call-instead-of-reading-the-resolved-class-cache) | Medium | Low |
29-
| P31 | [Reference index stores per-span entries when consumers only read distinct URIs](todo/performance.md#p31-reference-index-stores-per-span-entries-when-consumers-only-read-distinct-uris) | Medium | Low-Medium |
3029
| P38 | [Resolved type values are duplicated ~34x](todo/performance.md#p38-resolved-type-values-are-duplicated-34x) | High | High |
3130
| P39 | [`SymbolKind` stores owned strings per span](todo/performance.md#p39-symbolkind-stores-owned-strings-per-span) | Medium | Low-Medium |
3231
| P40 | [`method_index` is a per-class `HashMap` even when the member vec is shared](todo/performance.md#p40-method_index-is-a-per-class-hashmap-even-when-the-member-vec-is-shared) | Low-Medium | Low |
@@ -217,6 +216,7 @@ unlikely to move the needle for most users.
217216
| P21 | [Offset-shifting for cached diagnostics on partial edits](todo/performance.md#p21-offset-shifting-for-cached-diagnostics-on-partial-edits) | Medium | Medium |
218217
| P23 | [`workspace/symbol` lowercases every symbol name per request](todo/performance.md#p23-workspacesymbol-allocates-a-lowercase-copy-of-every-symbol-name-per-request) | Low-Medium | Low |
219218
| P28 | [`process_assert_narrowing` clones the scope per variable for every statement](todo/performance.md#p28-process_assert_narrowing-clones-the-top-level-scope-once-per-variable-for-every-statement) | Low-Medium | Low |
219+
| P42 | [Reference index is built during the headless `analyze` pipeline but never queried there](todo/performance.md#p42-reference-index-is-built-during-the-headless-analyze-pipeline-but-never-queried-there) | Low-Medium | Low |
220220
| | **[Indexing](todo/indexing.md)** | | |
221221
| X3 | Completion item detail on demand (`completionItem/resolve`) | Medium | Medium |
222222
| X7 | [Recency tracking](todo/indexing.md#x7-recency-tracking) | Medium | Medium |

docs/todo/performance.md

Lines changed: 23 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -831,56 +831,6 @@ waiting on the triggers above.
831831

832832
---
833833

834-
## P31. Reference index stores per-span entries when consumers only read distinct URIs
835-
836-
**Impact: Medium · Effort: Low-Medium**
837-
838-
The cross-file reference candidate index (`reference_index`) stores
839-
one entry per matching span, each with its own cloned URI `String`
840-
plus `start`/`end` offsets and an `is_declaration` flag. It has two
841-
consumers: `reference_candidate_uris_for_keys` reads only the
842-
distinct URIs per key, and the inlay-hint reference counts
843-
(`ref_count` in `src/inlay_hints.rs`) count the non-declaration
844-
entries per key. The `start`/`end` offsets are never read
845-
anywhere. Class and function spans additionally fan out into up
846-
to three name keys each (FQN, source spelling, short name), each with
847-
its own entry copy. On a large project this multiplies out to
848-
millions of heap-allocated URI strings held for the whole session —
849-
against the full-index memory aim — and the index is built on every
850-
`update_ast` in *all* indexing strategies and in the headless
851-
`analyze` pipeline, where it is never queried.
852-
853-
Two directions, pick one:
854-
855-
1. **Shrink to what is consumed.** Store per key the deduplicated
856-
URIs plus a non-declaration span count (what the inlay-hint
857-
consumer needs), e.g. `HashMap<Key, HashMap<Arc<str>, u32>>`
858-
with per-file URI interning so each file's URI is allocated
859-
once. Drop the unused offsets. Smallest change, keeps the
860-
current candidate-pruning design.
861-
2. **Make the spans pay for themselves.** Keep the per-span entries
862-
but have find-references consume them directly (the original
863-
O(k)-lookup goal), skipping the per-file symbol-map re-scan for
864-
keys that cannot produce false positives (constants, Laravel
865-
string keys). More work, bigger payoff.
866-
867-
Also worth fixing while in there: building entries calls
868-
`file_context_at` (which clones the file's full use map) once per
869-
`self`/`static`/`parent` span (and in fallback paths for class and
870-
function spans); only the enclosing class and namespace are needed.
871-
Skipping the index build entirely in the `analyze` pipeline is a
872-
free win under either direction.
873-
874-
**Quantified by the memory audit (see "Memory baseline" below):** on a
875-
large Laravel app the index holds 218,167 entries behind 74,454 distinct
876-
`(key, uri)` pairs — **2.9x span duplication** — costing 43 MB across
877-
330 K allocations. Direction 1 (dedup + count) collapses that to
878-
roughly the distinct-pair count, an estimated ~5 MB. Still worth doing,
879-
and now the third-largest single store after the resolved-class cache
880-
and `symbol_maps`.
881-
882-
---
883-
884834
## P32. Vendor package scan reads every file twice for origin classification
885835

886836
**Impact: Medium · Effort: Low-Medium**
@@ -916,7 +866,7 @@ set after the pass is ~1.16 GB, of which the resolved-class cache is
916866
**~748 MB — about 65%**. Ranked by the RSS each structure releases
917867
when dropped: resolved-class cache ~748 MB, class indexes
918868
(`method_store` + `fqn_class_index` + `uri_classes_index`) ~99 MB,
919-
`symbol_maps` ~26 MB, `reference_index` ~14 MB (see P31). The cache
869+
`symbol_maps` ~26 MB, `reference_index` ~14 MB. The cache
920870
holds ~247 KB per resolved class.
921871

922872
Two compounding causes:
@@ -1303,6 +1253,28 @@ the count itself has come down.
13031253

13041254
---
13051255

1256+
## P42. Reference index is built during the headless `analyze` pipeline but never queried there
1257+
1258+
**Impact: Low-Medium · Effort: Low**
1259+
1260+
`reindex_references_for_symbol_maps_batch` (`src/reference_index.rs`)
1261+
runs on every `update_ast`, including from the parallel index workers
1262+
in `src/analyse/run.rs` that back the headless `analyze` CLI
1263+
subcommand. That pipeline never calls
1264+
`reference_candidate_uris_for_keys` or any other reader of
1265+
`reference_index` (it has no find-references, rename, or inlay-hints
1266+
request to serve), so every span the workers walk to build index
1267+
entries there is wasted CPU and short-lived allocation.
1268+
1269+
Skip the build in that mode: add a per-`Backend` flag (not a
1270+
process-global static or thread-local, so test isolation between
1271+
`Backend` instances is preserved) that `Backend::new_headless` sets
1272+
and `reindex_references_for_symbol_maps_batch` checks before doing any
1273+
work, so the skip applies uniformly to every caller of `update_ast` in
1274+
headless mode rather than special-casing the `analyze` call site.
1275+
1276+
---
1277+
13061278
# Remaining anti-pattern fixes
13071279

13081280
Most remaining depth-cap issues were addressed by eager class

src/backend/file_access.rs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
//! lock-and-unwrap boilerplate that used to be duplicated across the
88
//! completion handler, definition resolver, and other consumers.
99
10+
use std::collections::HashMap;
1011
use std::sync::Arc;
1112

1213
use tower_lsp::lsp_types::Url;
@@ -195,6 +196,44 @@ impl Backend {
195196
}
196197
}
197198

199+
/// Subset of [`file_context_at`](Self::file_context_at) for callers
200+
/// that only need the enclosing class list and namespace (e.g.
201+
/// resolving `self`/`static`/`parent`). Skips the `use`-map and
202+
/// `resolved_names` clones.
203+
pub(crate) fn classes_and_namespace_at(
204+
&self,
205+
uri: &str,
206+
byte_offset: u32,
207+
) -> (Vec<Arc<ClassInfo>>, Option<String>) {
208+
let classes = self
209+
.symbols
210+
.uri_classes_index
211+
.read()
212+
.get(uri)
213+
.cloned()
214+
.unwrap_or_default();
215+
let namespace = self.namespace_at_offset(uri, byte_offset);
216+
(classes, namespace)
217+
}
218+
219+
/// Subset of [`file_context_at`](Self::file_context_at) for callers
220+
/// that only need the `use`-map and namespace (e.g. resolving a bare
221+
/// name to its FQN). Skips the class-list and `resolved_names` clones.
222+
pub(crate) fn use_map_and_namespace_at(
223+
&self,
224+
uri: &str,
225+
byte_offset: u32,
226+
) -> (HashMap<String, String>, Option<String>) {
227+
let use_map = self
228+
.file_imports
229+
.read()
230+
.get(uri)
231+
.cloned()
232+
.unwrap_or_default();
233+
let namespace = self.namespace_at_offset(uri, byte_offset);
234+
(use_map, namespace)
235+
}
236+
198237
/// Return the namespace that contains the given byte offset in a file.
199238
///
200239
/// For single-namespace files (the common case) this returns the file's

src/inlay_hints.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -403,7 +403,7 @@ impl Backend {
403403
self.reference_index
404404
.read()
405405
.get(key)
406-
.map(|entries| entries.iter().filter(|e| !e.is_declaration).count())
406+
.map(|entries| entries.values().map(|&count| count as usize).sum())
407407
.unwrap_or(0)
408408
}
409409

src/mem_audit.rs

Lines changed: 19 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1311,58 +1311,46 @@ pub(crate) fn report(backend: &Backend, runner_content_bytes: usize) {
13111311
);
13121312

13131313
// ── 6. Reference index ──────────────────────────────────────────
1314+
// Per key, only the distinct contributing URIs are stored (as a
1315+
// shared per-file `Arc<str>`) mapped to a non-declaration span
1316+
// count, so there is no per-span duplication left to account for.
13141317
let mut refs = Sz::default();
1315-
let mut n_entries = 0usize;
1318+
let mut n_key_uri_pairs = 0usize;
13161319
let n_keys;
1320+
let mut distinct_uris: HashSet<*const u8> = HashSet::new();
13171321
{
13181322
let idx = backend.reference_index.read();
13191323
let (by_key, uri_keys) = idx.audit_maps();
13201324
n_keys = by_key.len();
1321-
refs +=
1322-
map_buckets::<crate::reference_index::ReferenceIndexKey, Vec<u8>>(by_key.capacity());
1323-
for (k, v) in by_key {
1325+
refs += map_buckets::<crate::reference_index::ReferenceIndexKey, HashMap<Arc<str>, u32>>(
1326+
by_key.capacity(),
1327+
);
1328+
for (k, inner) in by_key {
13241329
refs.add(k.audit_heap());
1325-
n_entries += v.len();
1326-
refs.add(v.capacity() * size_of::<crate::reference_index::ReferenceIndexEntry>());
1327-
for e in v {
1328-
refs.add(e.uri.capacity());
1330+
n_key_uri_pairs += inner.len();
1331+
refs += map_buckets::<Arc<str>, u32>(inner.capacity());
1332+
for uri in inner.keys() {
1333+
distinct_uris.insert(Arc::as_ptr(uri).cast::<u8>());
13291334
}
13301335
}
1331-
refs += map_buckets::<String, Vec<crate::reference_index::ReferenceIndexKey>>(
1336+
refs += map_buckets::<Arc<str>, Vec<crate::reference_index::ReferenceIndexKey>>(
13321337
uri_keys.capacity(),
13331338
);
13341339
for (uri, keys) in uri_keys {
1335-
refs.add(uri.capacity());
1340+
refs.add(ARC + uri.len());
13361341
refs.add(keys.capacity() * size_of::<crate::reference_index::ReferenceIndexKey>());
13371342
for k in keys {
13381343
refs.add(k.audit_heap());
13391344
}
13401345
}
13411346
}
1342-
// How much of the index survives P31 direction 1: per key, keep the
1343-
// distinct URIs plus a non-declaration count, drop the never-read
1344-
// offsets and the per-span duplication.
1345-
let mut distinct_pairs = 0usize;
1346-
{
1347-
let idx = backend.reference_index.read();
1348-
let (by_key, _) = idx.audit_maps();
1349-
let mut uris: HashSet<&str> = HashSet::new();
1350-
for entries in by_key.values() {
1351-
uris.clear();
1352-
for e in entries {
1353-
uris.insert(e.uri.as_str());
1354-
}
1355-
distinct_pairs += uris.len();
1356-
}
1357-
}
13581347
eprintln!(
1359-
"── reference_index: {} keys, {} entries, {:.1} MB ({} allocs) | distinct (key, uri) pairs: {} ({:.1}x span duplication)",
1348+
"── reference_index: {} keys, {} (key, uri) pairs, {} distinct uris, {:.1} MB ({} allocs)",
13601349
n_keys,
1361-
n_entries,
1350+
n_key_uri_pairs,
1351+
distinct_uris.len(),
13621352
mb(refs.bytes),
13631353
refs.allocs,
1364-
distinct_pairs,
1365-
n_entries as f64 / distinct_pairs.max(1) as f64,
13661354
);
13671355

13681356
// ── 7. Remaining session stores ─────────────────────────────────
@@ -1569,7 +1557,7 @@ pub(crate) fn report(backend: &Backend, runner_content_bytes: usize) {
15691557
probe("file_imports", &mut || backend.file_imports.write().clear());
15701558
probe("symbol_maps", &mut || backend.symbol_maps.write().clear());
15711559
probe("reference_index", &mut || {
1572-
let uris: Vec<String> = backend
1560+
let uris: Vec<Arc<str>> = backend
15731561
.reference_index
15741562
.read()
15751563
.audit_maps()

0 commit comments

Comments
 (0)