Skip to content

Commit d00ac65

Browse files
AJenboAJenbo
committed
Eliminate ClassInfo cloning in completion pipeline
1 parent b755b53 commit d00ac65

10 files changed

Lines changed: 195 additions & 199 deletions

File tree

‎docs/CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2626
- **Closure and arrow function parameter inference.** Untyped closure and arrow function parameters are inferred from the enclosing call's callable signature, including through method chains that return `static`. Generic type substitution flows through to inferred parameters (e.g. a `callable(T)` parameter on a `Repository<Product>` resolves `T` to `Product`).
2727
- **Editing responsiveness.** Classes evicted from the resolved-class cache after a file edit are now eagerly re-populated in dependency order, eliminating lazy resolution delays for diagnostics and completions.
2828
- **Virtual member resolution.** Virtual member providers (PHPDoc mixins, Laravel model, Eloquent Builder) now resolve completely on every class, eliminating cases where mixins or Eloquent accessors were missing after edits.
29-
- **Faster analysis.** String interning, O(1) method lookup, and reduced redundant work cut analysis time significantly on large projects.
29+
- **Faster analysis.** String interning, O(1) method lookup, Arc-shared class metadata through the resolution pipeline, and reduced redundant work cut analysis time significantly on large projects.
3030

3131
### Fixed
3232

‎docs/todo/eager-resolution.md‎

Lines changed: 17 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ inside `collect_slow_diagnostics`. Added an early-return guard that
144144
skips the walk when the scope cache is already populated. This cut
145145
`slow` time from 9.4s to 0.6s on example.php.
146146

147-
#### Phase 4c: Eliminate ClassInfo cloning (next)
147+
#### Phase 4c: Eliminate ClassInfo cloning
148148

149149
**Impact: Critical. Effort: Medium-High.**
150150

@@ -175,25 +175,30 @@ the resolution pipeline.
175175

176176
**Plan:**
177177

178-
1. Change `ResolvedType.class_info` from `Option<ClassInfo>` to
178+
1. ✓ Change `ResolvedType.class_info` from `Option<ClassInfo>` to
179179
`Option<Arc<ClassInfo>>`. This propagates Arc sharing through
180-
the entire variable resolution pipeline.
180+
the entire variable resolution pipeline. Added `from_arc()` and
181+
`from_both_arc()` constructors for zero-clone creation.
181182

182-
2. Change `type_hint_to_classes_typed` to return
183+
2. ✓ Changed `type_hint_to_classes_typed` to return
183184
`Vec<Arc<ClassInfo>>`. Callers that need mutation (generic
184185
substitution, scope injection) clone only when necessary
185186
via `Arc::make_mut` or `Arc::unwrap_or_clone`.
186187

187-
3. Change `ResolvedType::into_classes` to return
188-
`Vec<Arc<ClassInfo>>` (rename to `into_arced_classes` or
189-
merge with existing method).
188+
3. ✓ `ResolvedType::into_arced_classes` now returns inner `Arc`s
189+
directly (zero-cost, no `Arc::new` wrapping). `into_classes`
190+
kept for the few callers needing owned `ClassInfo`.
190191

191-
4. Update `resolve_rhs_method_call_inner` and other RHS
192-
resolution functions to work with `&ClassInfo` references
193-
from the Arc instead of owned values.
192+
4. ✓ `resolve_rhs_method_call_inner` and `resolve_rhs_property_access`
193+
now use `Vec<Arc<ClassInfo>>` for owner classes, eliminating
194+
deep clones on every method/property access in the hot path.
195+
Remaining `Arc::unwrap_or_clone` sites (~40) are in cold paths
196+
(definition lookup, hover, narrowing) or require deeper
197+
signature changes.
194198

195-
5. Change `find_class_by_name` callers to use `Arc::clone`
196-
(refcount bump) instead of `ClassInfo::clone` (deep copy).
199+
5. ✓ `find_class_by_name` callers in `resolver.rs` now use
200+
`Arc::clone` + `from_arc` (refcount bump) instead of
201+
`ClassInfo::clone` (deep copy). 11 call sites converted.
197202

198203
**Expected impact:** Eliminates ~10-15% of total CPU (clone +
199204
drop + associated malloc/free). Combined with the double-walk

‎src/completion/call_resolution.rs‎

Lines changed: 13 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -180,7 +180,7 @@ impl Backend {
180180

181181
for rt in &resolved_types {
182182
let owner = match &rt.class_info {
183-
Some(ci) => Arc::new(ci.clone()),
183+
Some(ci) => Arc::clone(ci),
184184
None => continue,
185185
};
186186

@@ -808,10 +808,7 @@ impl Backend {
808808
owner_name,
809809
ctx.all_classes,
810810
ctx.class_loader,
811-
)
812-
.into_iter()
813-
.map(Arc::new)
814-
.collect();
811+
);
815812
if !classes.is_empty() {
816813
return classes;
817814
}
@@ -843,10 +840,7 @@ impl Backend {
843840
"",
844841
ctx.all_classes,
845842
ctx.class_loader,
846-
)
847-
.into_iter()
848-
.map(Arc::new)
849-
.collect();
843+
);
850844
if !classes.is_empty() {
851845
return classes;
852846
}
@@ -884,10 +878,7 @@ impl Backend {
884878
"",
885879
ctx.all_classes,
886880
ctx.class_loader,
887-
)
888-
.into_iter()
889-
.map(Arc::new)
890-
.collect();
881+
);
891882
if !classes.is_empty() {
892883
return classes;
893884
}
@@ -904,10 +895,7 @@ impl Backend {
904895
"",
905896
ctx.all_classes,
906897
ctx.class_loader,
907-
)
908-
.into_iter()
909-
.map(Arc::new)
910-
.collect();
898+
);
911899
}
912900
}
913901

@@ -932,10 +920,7 @@ impl Backend {
932920
"",
933921
ctx.all_classes,
934922
ctx.class_loader,
935-
)
936-
.into_iter()
937-
.map(Arc::new)
938-
.collect();
923+
);
939924
if !classes.is_empty() {
940925
return classes;
941926
}
@@ -955,10 +940,7 @@ impl Backend {
955940
"",
956941
ctx.all_classes,
957942
ctx.class_loader,
958-
)
959-
.into_iter()
960-
.map(Arc::new)
961-
.collect();
943+
);
962944
if !classes.is_empty() {
963945
return classes;
964946
}
@@ -974,10 +956,7 @@ impl Backend {
974956
"",
975957
ctx.all_classes,
976958
ctx.class_loader,
977-
)
978-
.into_iter()
979-
.map(Arc::new)
980-
.collect();
959+
);
981960
if !classes.is_empty() {
982961
return classes;
983962
}
@@ -999,10 +978,7 @@ impl Backend {
999978
"",
1000979
ctx.all_classes,
1001980
ctx.class_loader,
1002-
)
1003-
.into_iter()
1004-
.map(Arc::new)
1005-
.collect();
981+
);
1006982
if !classes.is_empty() {
1007983
return classes;
1008984
}
@@ -1044,10 +1020,7 @@ impl Backend {
10441020
"",
10451021
ctx.all_classes,
10461022
ctx.class_loader,
1047-
)
1048-
.into_iter()
1049-
.map(Arc::new)
1050-
.collect();
1023+
);
10511024
if !classes.is_empty() {
10521025
return classes;
10531026
}
@@ -1137,10 +1110,7 @@ impl Backend {
11371110
&class_info.fqn(),
11381111
all_classes,
11391112
class_loader,
1140-
)
1141-
.into_iter()
1142-
.map(Arc::new)
1143-
.collect();
1113+
);
11441114
if !classes.is_empty() {
11451115
return classes;
11461116
}
@@ -1162,10 +1132,7 @@ impl Backend {
11621132
&class_info.fqn(),
11631133
all_classes,
11641134
class_loader,
1165-
)
1166-
.into_iter()
1167-
.map(Arc::new)
1168-
.collect();
1135+
);
11691136
if !classes.is_empty() {
11701137
return classes;
11711138
}
@@ -1187,13 +1154,12 @@ impl Backend {
11871154
if ret.is_self_like() {
11881155
return vec![Arc::new(class_info.clone())];
11891156
}
1190-
let resolved = super::type_resolution::type_hint_to_classes_typed(
1157+
return super::type_resolution::type_hint_to_classes_typed(
11911158
ret,
11921159
&class_info.fqn(),
11931160
all_classes,
11941161
class_loader,
11951162
);
1196-
return resolved.into_iter().map(Arc::new).collect();
11971163
}
11981164
vec![]
11991165
};

‎src/completion/resolver.rs‎

Lines changed: 18 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -465,10 +465,10 @@ fn resolve_target_classes_expr_inner_impl(
465465
&& let Some(ref parent_name) = cc.parent_class
466466
{
467467
if let Some(cls) = find_class_by_name(all_classes, parent_name) {
468-
return vec![ResolvedType::from_class(cls.as_ref().clone())];
468+
return vec![ResolvedType::from_arc(Arc::clone(cls))];
469469
}
470470
return class_loader(parent_name)
471-
.map(|arc| ResolvedType::from_class(Arc::unwrap_or_clone(arc)))
471+
.map(ResolvedType::from_arc)
472472
.into_iter()
473473
.collect();
474474
}
@@ -533,7 +533,7 @@ fn resolve_target_classes_expr_inner_impl(
533533
);
534534
ResolvedType::extend_unique(
535535
&mut results,
536-
resolved.into_iter().map(ResolvedType::from_class).collect(),
536+
resolved.into_iter().map(ResolvedType::from_arc).collect(),
537537
);
538538
}
539539
if !results.is_empty() {
@@ -543,28 +543,28 @@ fn resolve_target_classes_expr_inner_impl(
543543

544544
owner_classes
545545
.into_iter()
546-
.map(|arc| ResolvedType::from_class(Arc::unwrap_or_clone(arc)))
546+
.map(ResolvedType::from_arc)
547547
.collect()
548548
}
549549

550550
// ── Bare class name ─────────────────────────────────────
551551
SubjectExpr::ClassName(name) => {
552552
if let Some(cls) = find_class_by_name(all_classes, name) {
553-
return vec![ResolvedType::from_class(cls.as_ref().clone())];
553+
return vec![ResolvedType::from_arc(Arc::clone(cls))];
554554
}
555555
class_loader(name)
556-
.map(|arc| ResolvedType::from_class(Arc::unwrap_or_clone(arc)))
556+
.map(ResolvedType::from_arc)
557557
.into_iter()
558558
.collect()
559559
}
560560

561561
// ── `new ClassName` (without trailing call parens) ───────
562562
SubjectExpr::NewExpr { class_name } => {
563563
if let Some(cls) = find_class_by_name(all_classes, class_name) {
564-
return vec![ResolvedType::from_class(cls.as_ref().clone())];
564+
return vec![ResolvedType::from_arc(Arc::clone(cls))];
565565
}
566566
class_loader(class_name)
567-
.map(|arc| ResolvedType::from_class(Arc::unwrap_or_clone(arc)))
567+
.map(ResolvedType::from_arc)
568568
.into_iter()
569569
.collect()
570570
}
@@ -584,15 +584,10 @@ fn resolve_target_classes_expr_inner_impl(
584584
if let Some(h) = hint
585585
&& classes.iter().any(|c| !c.template_params.is_empty())
586586
{
587-
let class_vec: Vec<ClassInfo> =
588-
classes.into_iter().map(Arc::unwrap_or_clone).collect();
589-
return ResolvedType::from_classes_with_hint(class_vec, h);
587+
return ResolvedType::from_classes_with_hint(classes, h);
590588
}
591589

592-
classes
593-
.into_iter()
594-
.map(|arc| ResolvedType::from_class(Arc::unwrap_or_clone(arc)))
595-
.collect()
590+
classes.into_iter().map(ResolvedType::from_arc).collect()
596591
}
597592

598593
// ── Property chain ──────────────────────────────────────
@@ -607,10 +602,7 @@ fn resolve_target_classes_expr_inner_impl(
607602
class_loader,
608603
);
609604

610-
ClassInfo::extend_unique_arc(
611-
&mut arc_results,
612-
resolved.into_iter().map(Arc::new).collect(),
613-
);
605+
ClassInfo::extend_unique_arc(&mut arc_results, resolved);
614606
}
615607

616608
// ── Property-level narrowing ────────────────────────
@@ -645,7 +637,7 @@ fn resolve_target_classes_expr_inner_impl(
645637

646638
arc_results
647639
.into_iter()
648-
.map(|arc| ResolvedType::from_class(Arc::unwrap_or_clone(arc)))
640+
.map(ResolvedType::from_arc)
649641
.collect()
650642
}
651643

@@ -669,7 +661,7 @@ fn resolve_target_classes_expr_inner_impl(
669661
class_loader,
670662
)
671663
{
672-
return resolved.into_iter().map(ResolvedType::from_class).collect();
664+
return resolved.into_iter().map(ResolvedType::from_arc).collect();
673665
}
674666
}
675667
// If raw-type approach didn't work, fall back to resolving
@@ -782,7 +774,7 @@ fn resolve_target_classes_expr_inner_impl(
782774
all_classes,
783775
class_loader,
784776
) {
785-
return resolved.into_iter().map(ResolvedType::from_class).collect();
777+
return resolved.into_iter().map(ResolvedType::from_arc).collect();
786778
}
787779
// Segment walk failed — the base type does not have
788780
// array-shape, generic, or iterable annotations that
@@ -803,10 +795,10 @@ fn resolve_target_classes_expr_inner_impl(
803795
| SubjectExpr::FunctionCall(_) => {
804796
let text = expr.to_subject_text();
805797
if let Some(cls) = find_class_by_name(all_classes, &text) {
806-
return vec![ResolvedType::from_class(cls.as_ref().clone())];
798+
return vec![ResolvedType::from_arc(Arc::clone(cls))];
807799
}
808800
class_loader(&text)
809-
.map(|arc| ResolvedType::from_class(Arc::unwrap_or_clone(arc)))
801+
.map(ResolvedType::from_arc)
810802
.into_iter()
811803
.collect()
812804
}
@@ -1262,7 +1254,7 @@ fn resolve_variable_fallback(
12621254
for cls in resolved {
12631255
ResolvedType::push_unique(
12641256
&mut class_string_results,
1265-
ResolvedType::from_class(cls),
1257+
ResolvedType::from_arc(cls),
12661258
);
12671259
}
12681260
}
@@ -1281,7 +1273,7 @@ fn resolve_variable_fallback(
12811273
for cls in resolved {
12821274
ResolvedType::push_unique(
12831275
&mut class_string_results,
1284-
ResolvedType::from_class(cls),
1276+
ResolvedType::from_arc(cls),
12851277
);
12861278
}
12871279
}

‎src/completion/source/helpers.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -495,7 +495,7 @@ pub(in crate::completion) fn try_chained_array_access_with_candidates<'a>(
495495
current_class: Option<&ClassInfo>,
496496
all_classes: &[Arc<ClassInfo>],
497497
class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
498-
) -> Option<Vec<ClassInfo>> {
498+
) -> Option<Vec<Arc<ClassInfo>>> {
499499
let current_class_name = current_class.map(|c| c.name.as_str()).unwrap_or("");
500500

501501
for candidate in candidates {
@@ -525,7 +525,7 @@ fn walk_array_segments_and_resolve(
525525
current_class_name: &str,
526526
all_classes: &[Arc<ClassInfo>],
527527
class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
528-
) -> Option<Vec<ClassInfo>> {
528+
) -> Option<Vec<Arc<ClassInfo>>> {
529529
// Expand type aliases before walking segments. The raw type may
530530
// be an alias name like `UserData` that resolves to
531531
// `array{name: string, pen: Pen}`. Without expansion the

0 commit comments

Comments
 (0)