Skip to content

Commit 3195f1e

Browse files
committed
Lower memory use for method lookups
1 parent 2679146 commit 3195f1e

5 files changed

Lines changed: 39 additions & 55 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
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.
4545
- **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.
46+
- **Lower memory use for method lookups.** Each resolved class's method name index is now a sorted list searched with binary search instead of a hash map, using about a third of the memory for the same lookup speed. On large Laravel projects, where the resolved-class cache holds thousands of these indexes, this measurably shrinks total memory use.
4647

4748
### Removed
4849

docs/todo.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ within the same impact tier.
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 |
2929
| P38 | [Resolved type values are duplicated ~34x](todo/performance.md#p38-resolved-type-values-are-duplicated-34x) | High | High |
30-
| 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 |
3130
| P41 | [3.8 M live allocations cost ~170 MB in allocator overhead](todo/performance.md#p41-38-m-live-allocations-cost-170-mb-in-allocator-overhead) | Medium | Medium |
3231
| P43 | [`MemberAccess.subject_text` still allocates a `String` per span](todo/performance.md#p43-memberaccesssubject_text-still-allocates-a-string-per-span) | Low-Medium | Medium |
3332
| P33 | [Workspace diagnostics leaves the whole project fully resolved in memory](todo/performance.md#p33-workspace-diagnostics-leaves-the-whole-project-fully-resolved-in-memory) | High | Medium-High |

docs/todo/performance.md

Lines changed: 4 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,33 +1156,6 @@ should keep owned payloads or use a separate bounded cache.
11561156

11571157
---
11581158

1159-
## P40. `method_index` is a per-class `HashMap` even when the member vec is shared
1160-
1161-
**Impact: Low-Medium · Effort: Low**
1162-
1163-
`ClassInfo::method_index` is an `AtomMap<u32>` rebuilt per resolved
1164-
class: 11 MB across 2 K classes on the mid-size app, 20 MB across 3.3 K
1165-
on the large one — roughly 6 KB per class. At ~214 members a class the
1166-
hashbrown bucket array rounds to 512 slots of 16 B plus control bytes,
1167-
while the data it indexes is 214 entries of 12 B.
1168-
1169-
The flattened member vectors are already shared between classes (only
1170-
8.6 MB of slot vectors back 3.3 K classes), but each class still builds
1171-
its own index over them.
1172-
1173-
Two options, either of which is small:
1174-
1175-
1. **Sorted `Vec<(Atom, u32)>` with binary search.** Roughly a third of
1176-
the memory (20 MB to ~7 MB) and better cache behaviour at these
1177-
sizes. Lookup goes from O(1) to O(log n) on ~200 entries, which is
1178-
3-4 comparisons of `Copy` pointer-sized keys.
1179-
2. **Share the index alongside the member vector.** When two classes
1180-
share a `SharedVec` of methods they can share one `Arc` of its index,
1181-
which keeps O(1) lookup and removes the duplication instead of
1182-
shrinking it.
1183-
1184-
---
1185-
11861159
## P41. 3.8 M live allocations cost ~170 MB in allocator overhead
11871160

11881161
**Impact: Medium · Effort: Medium**
@@ -1209,11 +1182,10 @@ trimmed live figure, confirming arena count was never the lever.
12091182

12101183
What is left is allocation *count* itself: at ~45 B of overhead per
12111184
live allocation, each allocation avoided is worth more than its own
1212-
`size_of`. P39 and P40 each remove hundreds of thousands of allocations
1213-
as a side effect (interning `SymbolKind` strings; sharing
1214-
`method_index`), so re-measure this line after those land. If a
1215-
meaningful gap remains afterward, the next lever is arena-allocating
1216-
the member metadata (bump-allocate `MethodInfo`/`PropertyInfo`/
1185+
`size_of`. P39 removes hundreds of thousands of allocations as a side
1186+
effect (interning `SymbolKind` strings), so re-measure this line after
1187+
it lands. If a meaningful gap remains afterward, the next lever is
1188+
arena-allocating the member metadata (bump-allocate `MethodInfo`/`PropertyInfo`/
12171189
`ParameterInfo` per resolved class instead of one `Box`/`Arc` per
12181190
member) — a much larger change than anything else in this list, so only
12191191
worth it if the per-allocation overhead is still the dominant cost once

src/mem_audit.rs

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -736,7 +736,9 @@ impl Audit {
736736
z.add(ARC + VEC_HDR + c.constants.audit_capacity() * size_of::<Arc<ConstantInfo>>());
737737
self.cls_slots += z;
738738
}
739-
self.cls_index += map_buckets::<Atom, u32>(c.method_index.capacity());
739+
let mut idx = Sz::default();
740+
idx.add(VEC_HDR + c.method_index.capacity() * size_of::<(Atom, u32)>());
741+
self.cls_index += idx;
740742

741743
let mut names = Sz::default();
742744
names.add(c.interfaces.capacity() * size_of::<Atom>());
@@ -948,11 +950,6 @@ impl Audit {
948950
self.n_paramvecs,
949951
mb(self.empty_param_vecs * (ARC + VEC_HDR)),
950952
);
951-
eprintln!(
952-
" method_index as sorted Vec<(Atom,u32)>: {:.1} MB → ~{:.1} MB",
953-
mb(self.cls_index.bytes),
954-
mb(self.cls_index.bytes / 3),
955-
);
956953
}
957954
}
958955

src/types/mod.rs

Lines changed: 31 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1579,15 +1579,22 @@ pub struct ClassInfo {
15791579
/// The outer [`SharedVec`] makes cloning the entire `ClassInfo`
15801580
/// O(1) (Arc refcount bump on the Vec itself).
15811581
pub methods: SharedVec<Arc<MethodInfo>>,
1582-
/// O(1) index from lowercased method name → position in `methods`
1583-
/// (PHP method names are case-insensitive).
1582+
/// Index from lowercased method name → position in `methods`
1583+
/// (PHP method names are case-insensitive), sorted by name for
1584+
/// binary search.
1585+
///
1586+
/// A sorted `Vec` costs about a third of the equivalent `AtomMap`
1587+
/// (hashbrown rounds its bucket array up to the next power of two,
1588+
/// so a class with ~200 methods pays for 512 slots) while a lookup
1589+
/// only costs a handful of `Atom` comparisons, which are `Copy`
1590+
/// pointer-sized values.
15841591
///
15851592
/// Rebuilt by [`rebuild_method_index`] after bulk mutations
15861593
/// (inheritance merge, parsing). The `get_method*` and `has_method`
1587-
/// helpers use this for O(1) lookup instead of linear scan.
1594+
/// helpers use this for `O(log n)` lookup instead of linear scan.
15881595
/// When empty or stale (detected via `indexed_method_count`),
15891596
/// the helpers fall back to linear scan.
1590-
pub method_index: AtomMap<u32>,
1597+
pub method_index: Vec<(Atom, u32)>,
15911598
/// The `methods.len()` at the time `method_index` was last built.
15921599
/// Used to detect staleness: if `methods.len() != indexed_method_count`,
15931600
/// the index is stale and the helpers fall back to linear scan.
@@ -1889,13 +1896,16 @@ impl ClassInfo {
18891896
self.method_index.clear();
18901897
self.method_index.reserve(self.methods.len());
18911898
for (i, method) in self.methods.iter().enumerate() {
1892-
// First-writer-wins: matches the semantics of
1893-
// `.iter().find(|m| m.name == name)` which returns the
1894-
// first match when duplicate names exist.
18951899
self.method_index
1896-
.entry(crate::atom::ascii_lowercase_atom(&method.name))
1897-
.or_insert(i as u32);
1900+
.push((crate::atom::ascii_lowercase_atom(&method.name), i as u32));
18981901
}
1902+
// Sort by name (then by original index) so lookups can binary
1903+
// search. Sorting the index pairs is stable per name, and
1904+
// deduping keeps the lowest index for each name, matching the
1905+
// first-writer-wins semantics of the old `.iter().find(...)`
1906+
// linear scan when duplicate names exist.
1907+
self.method_index.sort_unstable();
1908+
self.method_index.dedup_by_key(|&mut (name, _)| name);
18991909
self.indexed_method_count = self.methods.len() as u32;
19001910
}
19011911

@@ -1909,16 +1919,17 @@ impl ClassInfo {
19091919
/// Look up a method by name, ignoring ASCII case (PHP method names
19101920
/// are case-insensitive).
19111921
///
1912-
/// Uses the `method_index` for O(1) lookup when available,
1913-
/// falling back to linear scan otherwise.
1922+
/// Uses the `method_index` for `O(log n)` binary search when
1923+
/// available, falling back to linear scan otherwise.
19141924
#[inline]
19151925
pub fn get_method(&self, name: &str) -> Option<&MethodInfo> {
19161926
if self.method_index_valid() {
19171927
let atom = crate::atom::ascii_lowercase_atom(name);
19181928
return self
19191929
.method_index
1920-
.get(&atom)
1921-
.and_then(|&idx| self.methods.get(idx as usize))
1930+
.binary_search_by_key(&atom, |&(n, _)| n)
1931+
.ok()
1932+
.and_then(|pos| self.methods.get(self.method_index[pos].1 as usize))
19221933
.map(|arc| arc.as_ref());
19231934
}
19241935
self.methods
@@ -1940,7 +1951,10 @@ impl ClassInfo {
19401951
pub fn has_method(&self, name: &str) -> bool {
19411952
if self.method_index_valid() {
19421953
let atom = crate::atom::ascii_lowercase_atom(name);
1943-
return self.method_index.contains_key(&atom);
1954+
return self
1955+
.method_index
1956+
.binary_search_by_key(&atom, |&(n, _)| n)
1957+
.is_ok();
19441958
}
19451959
self.methods
19461960
.iter()
@@ -1959,8 +1973,9 @@ impl ClassInfo {
19591973
let atom = crate::atom::ascii_lowercase_atom(name);
19601974
return self
19611975
.method_index
1962-
.get(&atom)
1963-
.and_then(|&idx| self.methods.get(idx as usize))
1976+
.binary_search_by_key(&atom, |&(n, _)| n)
1977+
.ok()
1978+
.and_then(|pos| self.methods.get(self.method_index[pos].1 as usize))
19641979
.map(Arc::clone);
19651980
}
19661981
self.methods

0 commit comments

Comments
 (0)