Skip to content

Commit 62753f8

Browse files
committed
Property and constant sharing
1 parent d94d989 commit 62753f8

26 files changed

Lines changed: 440 additions & 147 deletions

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3636
### Changed
3737

3838
- **Lower memory use when resolving class hierarchies.** Resolving a class no longer copies every inherited or synthesized member into it. Members that a merge does not actually rewrite are shared with their source across the whole workspace, and the copies a merge does produce (template substitution, trait and interface merging, Eloquent Builder and scope forwarding) are deduplicated so that value-identical results share a single allocation instead of one per class. Method parameter lists are shared the same way. On large Laravel projects, where nearly every Eloquent model resolves through a generic base and forwards the same query-builder methods, this roughly halves the memory held by the resolved-class cache during whole-project analysis and speeds up the merge itself.
39+
- **Property and constant sharing extends the same memory win.** Inherited and synthesized properties and constants are now shared and interned the same way methods already were, instead of being cloned onto every class that inherits them. This further reduces the resolved-class cache's memory footprint on large projects, on top of the method sharing above.
3940
- **Classes are pre-resolved for the whole workspace after startup.** Once the background index completes, every known class is resolved in dependency order even when workspace diagnostics are disabled, so the first completion, hover, or go-to-definition against any class reads a warm cache instead of resolving on demand. Edits still re-resolve only the affected classes.
4041
- **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.
4142
- **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.

docs/todo/performance.md

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -913,18 +913,20 @@ holds ~247 KB per resolved class.
913913

914914
Two compounding causes:
915915

916-
1. **Inheritance flattening stores a full member copy per class.** Each
916+
1. **Inheritance flattening stores a member slot per class.** Each
917917
fully-resolved class stores the complete flattened member set (own +
918-
trait + parent + interface + mixin). On the measured project that is
919-
648 K method slots across 3 K classes (~214 methods/class), even
920-
though only ~20 K unique `MethodInfo` values exist in `method_store`.
921-
`resolve_class_with_inheritance` (`inheritance.rs`) shares the
922-
`Arc<MethodInfo>` for any inherited method that does not mention a
923-
substituted template parameter, so most slots point back at the base
924-
class, but every method that does reference a template param (and
925-
every class that overrides or enriches one) still holds a distinct
926-
allocation. The flattening itself is what the separated-metadata fix
927-
below would eliminate.
918+
trait + parent + interface + mixin), so a project with ~3 K classes
919+
at ~214 members each holds ~648 K method slots. The distinct
920+
*allocations* behind those slots are now heavily shared for methods,
921+
properties, and constants alike: a slot the merge does not rewrite
922+
points back at its source `Arc<MethodInfo>`/`Arc<PropertyInfo>`/
923+
`Arc<ConstantInfo>`, and the copies a merge does produce (template
924+
substitution, trait / interface / enrichment merges, Eloquent Builder
925+
and scope forwarding) are interned so value-identical results
926+
collapse to one allocation with a shared parameter list. What remains
927+
per class is the flattened `Vec<Arc<_>>` itself — one pointer slot
928+
per visible member — which the separated-metadata fix below would
929+
eliminate.
928930

929931
2. **The cache is retained for the whole session, unbounded.** Eager
930932
population plus lazy resolution during the pass fill the cache with
@@ -943,14 +945,19 @@ Fixes, cheapest first (each stands alone):
943945
cycles are broken by the cache's in-flight set, which `clear()`
944946
deliberately leaves untouched, so clearing mid-resolution cannot
945947
resurrect unbounded recursion. **Low-Medium.**
946-
2. **Separated metadata.** The structural fix: store method
947-
identifier `Atom`s per class and keep a single global `MethodInfo`
948-
store, resolving members on demand instead of flattening a cloned copy
949-
into every class (Mago's model). Eliminates the flattening entirely.
950-
This was the endgame of the (now-completed) eager-resolution track;
951-
profiling closed it as not worth the rewrite for analysis *speed*,
952-
but it remains a valid direction for *memory* if the cache bound above
953-
proves insufficient. **High.**
948+
2. **Separated metadata.** The structural fix for the remaining
949+
per-class pointer vectors: store member identifier `Atom`s per class
950+
and keep a single global member store, resolving members on demand
951+
instead of flattening a per-class `Vec` of pointers (Mago's model).
952+
With member *allocations* already shared and interned across methods,
953+
properties, and constants, the win left here is the pointer slots
954+
themselves. This needs substitution moved to query time to fit
955+
PHPantom's transform-on-inherit model (an inherited member is not
956+
byte-identical to the declaring class's once template substitution or
957+
bare-`self` rewriting applies, so a pure `(fqn, name)` redirect would
958+
lose the transform). Profiling closed this as not worth the rewrite
959+
for analysis *speed*; it remains a valid direction for *memory* if the
960+
cache bound above proves insufficient. **High.**
954961

955962
---
956963

src/completion/context/override_completion.rs

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -312,7 +312,7 @@ pub(crate) fn collect_overridable_properties(
312312
if own.contains(&lower) || !seen.insert(lower) {
313313
continue;
314314
}
315-
results.push((prop.clone(), declaring.clone(), false));
315+
results.push(((**prop).clone(), declaring.clone(), false));
316316
}
317317
let mut collector = PropertyCollector {
318318
partial,
@@ -384,7 +384,7 @@ impl PropertyCollector<'_> {
384384
continue;
385385
}
386386
self.results
387-
.push((prop.clone(), declaring.clone(), self.skip_override_attr));
387+
.push(((**prop).clone(), declaring.clone(), self.skip_override_attr));
388388
}
389389
}
390390
}
@@ -428,7 +428,7 @@ pub(crate) fn collect_overridable_constants(
428428
if own.contains(&lower) || !seen.insert(lower) {
429429
continue;
430430
}
431-
results.push((c.clone(), declaring.clone()));
431+
results.push(((**c).clone(), declaring.clone()));
432432
}
433433
parent_name = parent.parent_class;
434434
depth += 1;
@@ -949,7 +949,7 @@ mod tests {
949949
fn collects_parent_constants() {
950950
let mut base = make_class("Base");
951951
base.constants = vec![
952-
ConstantInfo {
952+
Arc::new(ConstantInfo {
953953
name: atom("STATUS_OK"),
954954
name_offset: 0,
955955
type_hint: None,
@@ -962,8 +962,8 @@ mod tests {
962962
enum_value: None,
963963
value: Some("1".into()),
964964
is_virtual: false,
965-
},
966-
ConstantInfo {
965+
}),
966+
Arc::new(ConstantInfo {
967967
name: atom("SECRET"),
968968
name_offset: 0,
969969
type_hint: None,
@@ -976,7 +976,7 @@ mod tests {
976976
enum_value: None,
977977
value: None,
978978
is_virtual: false,
979-
},
979+
}),
980980
]
981981
.into();
982982

src/hover/class.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,7 +157,12 @@ const MAX_BODY_ITEMS: usize = 30;
157157
/// If there are more than [`MAX_BODY_ITEMS`] cases, the list is truncated
158158
/// with a `// and N more…` comment.
159159
fn build_enum_case_body(cls: &ClassInfo) -> String {
160-
let cases: Vec<&ConstantInfo> = cls.constants.iter().filter(|c| c.is_enum_case).collect();
160+
let cases: Vec<&ConstantInfo> = cls
161+
.constants
162+
.iter()
163+
.filter(|c| c.is_enum_case)
164+
.map(|c| c.as_ref())
165+
.collect();
161166

162167
if cases.is_empty() {
163168
return String::new();

src/hover/member.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -446,10 +446,10 @@ impl Backend {
446446
.get_method_ci(member_name)
447447
.map(|m| HoverMemberHit::Method(Box::new(m.clone())))
448448
} else {
449-
if let Some(prop) = class.properties.iter().find(|p| p.name == member_name) {
449+
if let Some(prop) = class.get_property(member_name) {
450450
return Some(HoverMemberHit::Property(Box::new(prop.clone())));
451451
}
452-
if let Some(constant) = class.constants.iter().find(|c| c.name == member_name) {
452+
if let Some(constant) = class.get_constant(member_name) {
453453
return Some(HoverMemberHit::Constant(Box::new(constant.clone())));
454454
}
455455
class

src/inheritance/enrichment.rs

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -327,6 +327,38 @@ pub(crate) fn enrich_single_parameter(
327327
/// Propagates type hints and descriptions from the ancestor when the
328328
/// child lacks its own docblock overrides. The same
329329
/// effective-vs-native comparison is used as for method return types.
330+
/// Copy-on-write wrapper around [`enrich_property_from_ancestor`].
331+
///
332+
/// The property analogue of [`enrich_method_arc_from_ancestor`]: the
333+
/// shared `Arc<PropertyInfo>` is only deep-cloned when the enrichment
334+
/// would actually change a field, which keeps inherited properties
335+
/// sharing their allocation in the common no-op case.
336+
pub(crate) fn enrich_property_arc_from_ancestor(
337+
existing: &mut Arc<PropertyInfo>,
338+
ancestor: &PropertyInfo,
339+
) {
340+
if property_enrichment_would_change(existing, ancestor) {
341+
enrich_property_from_ancestor(Arc::make_mut(existing), ancestor);
342+
}
343+
}
344+
345+
/// Whether [`enrich_property_from_ancestor`] would change any field of
346+
/// `existing`. Mirrors the apply function's conditions exactly (keep the
347+
/// two in sync); an overwrite with an equal value counts as "no change".
348+
fn property_enrichment_would_change(existing: &PropertyInfo, ancestor: &PropertyInfo) -> bool {
349+
// Type hint.
350+
if (existing.type_hint.is_none() && ancestor.type_hint.is_some()
351+
|| lacks_docblock_override(&existing.type_hint, &existing.native_type_hint)
352+
&& ancestor_has_richer_type(&ancestor.type_hint, &ancestor.native_type_hint))
353+
&& existing.type_hint != ancestor.type_hint
354+
{
355+
return true;
356+
}
357+
358+
// Description.
359+
existing.description.is_none() && ancestor.description.is_some()
360+
}
361+
330362
pub(crate) fn enrich_property_from_ancestor(existing: &mut PropertyInfo, ancestor: &PropertyInfo) {
331363
// ── Type hint ───────────────────────────────────────────────
332364
// Same logic as method return types: propagate when the child

src/inheritance/generics.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -393,9 +393,16 @@ pub(crate) fn apply_generic_args(class: &ClassInfo, type_args: &[PhpType]) -> Cl
393393
.iter()
394394
.any(|p| property_references_params(p, &sub_keys))
395395
{
396+
let fp = crate::virtual_members::TransformFingerprint::new(Some(&subs), None, 0);
396397
for property in result.properties.make_mut() {
397398
if property_references_params(property, &sub_keys) {
398-
apply_substitution_to_property(property, &subs);
399+
let transformed =
400+
crate::virtual_members::intern_transformed_property(property, fp, || {
401+
let mut p = (**property).clone();
402+
apply_substitution_to_property(&mut p, &subs);
403+
p
404+
});
405+
*property = transformed;
399406
}
400407
}
401408
}

src/inheritance/mod.rs

Lines changed: 37 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -27,11 +27,13 @@ use std::sync::Arc;
2727
use crate::atom::{Atom, AtomSet, atom};
2828
use crate::php_type::PhpType;
2929
use crate::types::{ClassInfo, MAX_INHERITANCE_DEPTH, Visibility};
30-
use crate::virtual_members::{TransformFingerprint, intern_transformed_method};
30+
use crate::virtual_members::{
31+
TransformFingerprint, intern_transformed_method, intern_transformed_property,
32+
};
3133

3234
// Re-export functions that are used internally
3335
pub(crate) use enrichment::enrich_method_arc_from_ancestor;
34-
pub(crate) use enrichment::enrich_property_from_ancestor;
36+
pub(crate) use enrichment::enrich_property_arc_from_ancestor;
3537

3638
#[cfg(test)]
3739
pub(crate) use generics::apply_substitution;
@@ -405,15 +407,16 @@ pub(crate) fn resolve_class_with_inheritance(
405407
merged.methods.push(transformed);
406408
}
407409

408-
// Merge parent properties — same enrichment logic.
410+
// Merge parent properties — same enrichment logic as methods.
411+
// The substitution transform reuses `fp_sub` (a property and a
412+
// method with the same substitution map fingerprint to the same
413+
// value, but they intern into separate maps keyed by origin type,
414+
// so there is no collision).
409415
for property in &parent.properties {
410416
if property.visibility == Visibility::Private {
411417
continue;
412418
}
413-
let mut ancestor_property = property.clone();
414-
if property_references_params(property, &sub_keys) {
415-
apply_substitution_to_property(&mut ancestor_property, &level_subs);
416-
}
419+
let needs_sub = property_references_params(property, &sub_keys);
417420
if !dedup.properties.insert(property.name) {
418421
// Child already has this property — enrich it from parent.
419422
if let Some(existing) = merged
@@ -422,22 +425,44 @@ pub(crate) fn resolve_class_with_inheritance(
422425
.iter_mut()
423426
.find(|p| p.name == property.name)
424427
{
425-
enrich_property_from_ancestor(existing, &ancestor_property);
428+
if needs_sub {
429+
let ancestor_property =
430+
intern_transformed_property(property, fp_sub, || {
431+
let mut p = (**property).clone();
432+
apply_substitution_to_property(&mut p, &level_subs);
433+
p
434+
});
435+
enrich_property_arc_from_ancestor(existing, &ancestor_property);
436+
} else {
437+
enrich_property_arc_from_ancestor(existing, property);
438+
}
426439
}
427440
continue;
428441
}
429-
merged.properties.push(ancestor_property);
442+
if !needs_sub {
443+
// Substitution is a no-op: keep the shared `Arc`.
444+
merged.properties.push(Arc::clone(property));
445+
continue;
446+
}
447+
let transformed = intern_transformed_property(property, fp_sub, || {
448+
let mut p = (**property).clone();
449+
apply_substitution_to_property(&mut p, &level_subs);
450+
p
451+
});
452+
merged.properties.push(transformed);
430453
}
431454

432-
// Merge parent constants
455+
// Merge parent constants. Constants are never transformed on
456+
// inheritance, so an inherited constant is always a plain
457+
// `Arc::clone` of the declaring class's.
433458
for constant in &parent.constants {
434459
if constant.visibility == Visibility::Private {
435460
continue;
436461
}
437462
if !dedup.constants.insert(constant.name) {
438463
continue;
439464
}
440-
merged.constants.push(constant.clone());
465+
merged.constants.push(Arc::clone(constant));
441466
}
442467

443468
// Carry the substitution map forward for the next level.
@@ -501,7 +526,7 @@ pub(crate) fn resolve_class_with_inheritance(
501526
.iter_mut()
502527
.find(|p| p.name == "value")
503528
{
504-
prop.type_hint = Some(specific_type);
529+
Arc::make_mut(prop).type_hint = Some(specific_type);
505530
}
506531
}
507532

src/inheritance/traits.rs

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,13 @@ use crate::util::short_name;
1414
use crate::virtual_members::laravel::{
1515
extends_eloquent_model, is_has_factory_trait, model_to_factory_fqn,
1616
};
17-
use crate::virtual_members::{TransformFingerprint, intern_transformed_method};
17+
use crate::virtual_members::{
18+
TransformFingerprint, intern_transformed_method, intern_transformed_property,
19+
};
1820

1921
use super::generics::{
2022
apply_substitution_to_method, apply_substitution_to_property, method_references_params,
21-
right_align_offset,
23+
property_references_params, right_align_offset,
2224
};
2325
use super::{MergeDedup, TraitContext};
2426

@@ -175,7 +177,7 @@ pub(crate) fn merge_traits_into(
175177
if !dedup.properties.insert(property.name) {
176178
continue;
177179
}
178-
merged.properties.push(property.clone());
180+
merged.properties.push(Arc::clone(property));
179181
}
180182

181183
// Merge parent constants
@@ -186,7 +188,7 @@ pub(crate) fn merge_traits_into(
186188
if !dedup.constants.insert(constant.name) {
187189
continue;
188190
}
189-
merged.constants.push(constant.clone());
191+
merged.constants.push(Arc::clone(constant));
190192
}
191193

192194
current = parent;
@@ -288,24 +290,30 @@ pub(crate) fn merge_traits_into(
288290
}
289291
}
290292

291-
// Merge trait properties — apply substitution.
293+
// Merge trait properties — apply substitution, sharing the `Arc`
294+
// (or the interned transformed copy) as the method loop does.
292295
for property in &trait_info.properties {
293296
if !dedup.properties.insert(property.name) {
294297
continue;
295298
}
296-
let mut property = property.clone();
297-
if !trait_subs.is_empty() {
298-
apply_substitution_to_property(&mut property, &trait_subs);
299+
if !property_references_params(property, &sub_keys) {
300+
merged.properties.push(Arc::clone(property));
301+
continue;
299302
}
300-
merged.properties.push(property);
303+
let transformed = intern_transformed_property(property, fp_sub, || {
304+
let mut p = (**property).clone();
305+
apply_substitution_to_property(&mut p, &trait_subs);
306+
p
307+
});
308+
merged.properties.push(transformed);
301309
}
302310

303-
// Merge trait constants
311+
// Merge trait constants — never transformed, plain `Arc::clone`.
304312
for constant in &trait_info.constants {
305313
if !dedup.constants.insert(constant.name) {
306314
continue;
307315
}
308-
merged.constants.push(constant.clone());
316+
merged.constants.push(Arc::clone(constant));
309317
}
310318

311319
// Apply `as` alias declarations that create new method names.

src/parser/anonymous.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,8 +77,16 @@ impl Backend {
7777
kind: ClassLikeKind::Class,
7878
name,
7979
methods: methods.into_iter().map(Arc::new).collect::<Vec<_>>().into(),
80-
properties: properties.into(),
81-
constants: constants.into(),
80+
properties: properties
81+
.into_iter()
82+
.map(Arc::new)
83+
.collect::<Vec<_>>()
84+
.into(),
85+
constants: constants
86+
.into_iter()
87+
.map(Arc::new)
88+
.collect::<Vec<_>>()
89+
.into(),
8290
start_offset,
8391
end_offset,
8492
keyword_offset,

0 commit comments

Comments
 (0)