Finish resolved_class_cache generic-arg specialisation

Author: AJenbo

docs/todo.md

@@ -25,10 +25,9 @@ within the same impact tier.
 
 | #   | Item                                                                                                                  | Impact     | Effort |
 | --- | --------------------------------------------------------------------------------------------------------------------- | ---------- | ------ |
-|     | Clear [refactoring gate](todo/refactor.md)                                                                                                                      | —           | —           |
+| ER5 | [Mago-style separated metadata](todo/eager-resolution.md#er5-mago-style-separated-metadata)                                                                                | High        | High        |
 | 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  |
 | 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 |
-| P9  | [`resolved_class_cache` generic-arg specialisation](todo/performance.md#p9-resolved_class_cache-generic-arg-specialisation) | Medium      | Medium      |
 | X10 | [Interactive requests block on the workspace index lock during initial indexing](todo/indexing.md#x10-interactive-requests-block-on-the-workspace-index-lock-during-initial-indexing) | Medium      | Medium      |
 | L21 | [Tighten the supertype-where-subtype comparison escape hatch (blocked on resolver precision)](todo/laravel.md#l21-tighten-the-supertype-where-subtype-comparison-escape-hatch-blocked-on-resolver-precision) | Medium      | High        |
 | L18 | [`Macroable::mixin()` registrations](todo/laravel.md)                                                                          | Low         | Medium      |
@@ -197,14 +196,12 @@ unlikely to move the needle for most users.
 | E6  | Stub install prompt for non-Composer projects                                                                                                                               | Low         | Low         |
 | E7  | [Stub-based framework patches](todo/external-stubs.md#e7-stub-based-framework-patches)                                                                                      | Medium      | Medium      |
 |     | **[Performance](todo/performance.md) / [Eager Resolution](todo/eager-resolution.md)**                                                                                       |             |             |
-| ER5 | [Mago-style separated metadata](todo/eager-resolution.md#er5-mago-style-separated-metadata)                                                                                | High        | High        |
 | P29 | [Migrate to `mago-phpdoc-syntax`](todo/performance.md#p29-migrate-to-mago-phpdoc-syntax) (drop deprecated `mago-docblock` / `mago-type-syntax`)                             | Medium      | Medium      |
 | P30 | [Evaluate migrating parse/resolve/docblock pipeline to `mago-hir`](todo/performance.md#p30-evaluate-migrating-parseresolvedocblock-pipeline-to-mago-hir) (blocked on upstream API stabilizing — see triggers) | Medium-High | High        |
 | P16 | [Pre-parsed stub format (eliminate raw PHP embedding)](todo/performance.md#p16-pre-parsed-stub-format-eliminate-raw-php-embedding)                                          | High        | Medium-High |
 | P25 | [`type_mismatch_argument` / `argument_count_mismatch` slow on large single files](todo/performance.md#p25-type_mismatch_argument-argument_count_mismatch-slow-on-large-single-files) | Medium      | Medium      |
 | P22 | [Signature change re-queues slow diagnostics for every open file](todo/performance.md#p22-signature-change-re-queues-slow-diagnostics-for-every-open-file)                  | Medium-High | Medium      |
 | P14 | [Eager docblock parsing into structured fields](todo/performance.md#p14-eager-docblock-parsing-into-structured-fields)                                                      | Medium      | Medium      |
-| P9  | [`resolved_class_cache` generic-arg specialisation](todo/performance.md#p9-resolved_class_cache-generic-arg-specialisation)                                                 | Medium      | Medium      |
 | P27 | [`object`/`?object` call-return check re-resolves the subject a second time](todo/performance.md#p27-objectobject-call-return-check-re-resolves-the-subject-a-second-time)  | Medium      | Low         |
 | P11 | [Uncached base-resolution in `build_scope_methods_for_builder`](todo/performance.md#p11-uncached-base-resolution-in-build_scope_methods_for_builder)                        | Low-Medium  | Low         |
 | P3  | Parallel pre-filter in `find_implementors`                                                                                                                                  | Low-Medium  | Medium      |

docs/todo/performance.md

@@ -51,36 +51,6 @@ threshold (e.g. 8 files).
 
 ---
 
-## P9. `resolved_class_cache` generic-arg specialisation
-
-**Impact: Medium · Effort: Medium**
-
-The resolved-class cache is keyed by `(FQN, Vec<String>)`. Every
-distinct generic instantiation of the same class (e.g.
-`Builder<User>`, `Builder<Order>`, `Builder<Product>`) triggers a
-full `resolve_class_fully` call, even though the base resolution
-(inheritance merging, trait merging, virtual member injection) is
-identical. Only the final generic substitution differs.
-
-In a Laravel codebase with hundreds of Eloquent models, this means
-`Builder` is fully resolved hundreds of times, once per model.
-
-### Fix
-
-Cache the base-resolved class (before generic substitution)
-separately, keyed by FQN alone. When a generic instantiation is
-requested, look up the base-resolved class and apply
-`apply_substitution` on top. The substitution step is cheap
-(tree walk) compared to the full resolution (inheritance walking,
-trait merging, virtual member providers).
-
-This requires splitting `resolve_class_fully` into two stages:
-base resolution (cached by FQN) and generic specialisation (cached
-by `(FQN, Vec<String>)` as today, but with a much cheaper miss
-path).
-
----
-
 ## P11. Uncached base-resolution in `build_scope_methods_for_builder`
 
 **Impact: Low-Medium · Effort: Low**

src/virtual_members/resolve.rs

@@ -115,7 +115,7 @@ pub fn populate_from_sorted(
         //    for related classes, which hit the cache (already populated)
         // 4. Merge interfaces — also hit cache for resolved interfaces
         // 5. Store result in cache
-        resolve_class_fully_inner(&raw_class, class_loader, Some(cache), &[]);
+        resolve_class_fully_inner(&raw_class, class_loader, Some(cache));
     }
 }
 
@@ -146,7 +146,7 @@ pub fn resolve_class_fully(
     class: &ClassInfo,
     class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
 ) -> Arc<ClassInfo> {
-    resolve_class_fully_inner(class, class_loader, None, &[])
+    resolve_class_fully_inner(class, class_loader, None)
 }
 
 /// Cached variant of [`resolve_class_fully`].
@@ -166,7 +166,7 @@ pub fn resolve_class_fully_cached(
     class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
     cache: &ResolvedClassCache,
 ) -> Arc<ClassInfo> {
-    resolve_class_fully_inner(class, class_loader, Some(cache), &[])
+    resolve_class_fully_inner(class, class_loader, Some(cache))
 }
 
 /// Resolve a class fully, using the cache when available.
@@ -182,7 +182,7 @@ pub fn resolve_class_fully_maybe_cached(
     cache: Option<&ResolvedClassCache>,
 ) -> Arc<ClassInfo> {
     let started = std::time::Instant::now();
-    let result = resolve_class_fully_inner(class, class_loader, cache, &[]);
+    let result = resolve_class_fully_inner(class, class_loader, cache);
     let elapsed = started.elapsed();
     if elapsed >= std::time::Duration::from_millis(50) {
         tracing::info!(
@@ -220,7 +220,7 @@ pub fn resolve_class_fully_with_generics(
 ) -> Arc<ClassInfo> {
     // Fast path: no generics — just do the base resolution.
     if generic_args.is_empty() {
-        return resolve_class_fully_inner(class, class_loader, cache, &[]);
+        return resolve_class_fully_inner(class, class_loader, cache);
     }
 
     // Check the cache for (FQN, generic_args).
@@ -236,7 +236,7 @@ pub fn resolve_class_fully_with_generics(
 
     let started = std::time::Instant::now();
     // Resolve the base class (cached at (FQN, [])).
-    let base = resolve_class_fully_inner(class, class_loader, cache, &[]);
+    let base = resolve_class_fully_inner(class, class_loader, cache);
 
     // Apply generic substitution.
     let result = if !base.template_params.is_empty() {
@@ -278,7 +278,7 @@ pub fn resolve_class_fully_with_type_args(
     generic_args: &[crate::php_type::PhpType],
 ) -> Arc<ClassInfo> {
     if generic_args.is_empty() {
-        return resolve_class_fully_inner(class, class_loader, cache, &[]);
+        return resolve_class_fully_inner(class, class_loader, cache);
     }
 
     let generic_arg_strings: Vec<String> = generic_args.iter().map(|arg| arg.to_string()).collect();
@@ -293,14 +293,23 @@ pub fn resolve_class_fully_with_type_args(
 
 /// Shared implementation behind [`resolve_class_fully`] and
 /// [`resolve_class_fully_cached`].
+///
+/// This always produces the *base* resolution, cached under the bare
+/// FQN key `(FQN, [])`. Generic specialisation is a separate, much
+/// cheaper stage layered on top by
+/// [`resolve_class_fully_with_generics`]: it looks up this base result
+/// and applies [`apply_generic_args`](crate::inheritance::apply_generic_args),
+/// caching the substituted class under `(FQN, generic_args)`. Keeping the
+/// two stages distinct means the expensive inheritance/trait/virtual-member
+/// merge runs once per class, no matter how many generic instantiations
+/// (`Builder<User>`, `Builder<Order>`, …) are requested.
 fn resolve_class_fully_inner(
     class: &ClassInfo,
     class_loader: &dyn Fn(&str) -> Option<Arc<ClassInfo>>,
     cache: Option<&ResolvedClassCache>,
-    generic_args: &[String],
 ) -> Arc<ClassInfo> {
     let fqn = class.fqn();
-    let cache_key: ResolvedClassCacheKey = (fqn, generic_args.to_vec());
+    let cache_key: ResolvedClassCacheKey = (fqn, Vec::new());
 
     // ── Reload raw class from class_loader ──────────────────────────
     // Callers sometimes pass a ClassInfo that has already been through

src/virtual_members/tests.rs

@@ -1065,6 +1065,81 @@ fn resolve_class_fully_with_type_args_caches_specialisation() {
     );
 }
 
+#[test]
+fn base_resolution_shared_across_distinct_specialisations() {
+    // The point of the two-stage split: the expensive base resolution
+    // (inheritance/trait/virtual-member merge) runs once per class, no
+    // matter how many distinct generic instantiations are requested. A
+    // parent that the inheritance walk must load lets us count whether
+    // the base merge re-ran: on a cached base hit the parent is not
+    // loaded again.
+    let mut base = make_class("Base");
+    base.methods
+        .push(Arc::new(make_method("save", Some("bool"))));
+
+    let mut collection = make_class("Collection");
+    collection.template_params.push(atom("TValue"));
+    collection.parent_class = Some(atom("Base"));
+    collection
+        .methods
+        .push(Arc::new(make_method("first", Some("TValue"))));
+
+    let base_arc = Arc::new(base);
+    let collection_arc = Arc::new(collection);
+    let base_loads = std::cell::Cell::new(0u32);
+    let class_loader = |name: &str| -> Option<Arc<crate::types::ClassInfo>> {
+        match name {
+            "Base" => {
+                base_loads.set(base_loads.get() + 1);
+                Some(Arc::clone(&base_arc))
+            }
+            "Collection" => Some(Arc::clone(&collection_arc)),
+            _ => None,
+        }
+    };
+
+    let cache = new_resolved_class_cache();
+
+    // First specialisation triggers the one-and-only base resolution.
+    let user = resolve_class_fully_with_type_args(
+        &collection_arc,
+        &class_loader,
+        Some(&cache),
+        &[PhpType::parse("User")],
+    );
+    let after_first = base_loads.get();
+    assert!(after_first > 0, "base resolution must load the parent");
+
+    // A different specialisation must reuse the cached base, loading the
+    // parent zero additional times.
+    let order = resolve_class_fully_with_type_args(
+        &collection_arc,
+        &class_loader,
+        Some(&cache),
+        &[PhpType::parse("Order")],
+    );
+    assert_eq!(
+        base_loads.get(),
+        after_first,
+        "distinct generic specialisation must not re-run base resolution"
+    );
+
+    // Each specialisation still substitutes its own type argument.
+    assert_eq!(
+        user.get_method("first").and_then(|m| m.return_type_str()),
+        Some("User".to_string())
+    );
+    assert_eq!(
+        order.get_method("first").and_then(|m| m.return_type_str()),
+        Some("Order".to_string())
+    );
+
+    let map = cache.read();
+    assert!(map.contains_key(&(atom("Collection"), Vec::new())));
+    assert!(map.contains_key(&(atom("Collection"), vec!["User".to_string()])));
+    assert!(map.contains_key(&(atom("Collection"), vec!["Order".to_string()])));
+}
+
 #[test]
 fn evict_removes_direct_match() {
     let mut cache = make_cache();