Whole-workspace class pre-resolution now spreads across cores

Author: AJenbo

docs/CHANGELOG.md

@@ -54,6 +54,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **Laravel string keys are collected once instead of once per CPU core.** Checking a `route()`, `config()`, `view()`, or `__()` key needs the project's full list of valid keys, and building each list walks the project directory. The diagnostic pass runs one worker per core, so all of them used to reach the empty list at the same moment and every one repeated the same walk, leaving most cores waiting on the disk rather than analysing. Each list is now built once and shared, which roughly doubles the number of cores the diagnostic pass keeps busy and cuts whole-project analysis time by around 15% on large Laravel projects.
 - **Repeated class lookups are remembered instead of searched again.** Answering "which class does this name refer to?" is the single most frequent thing PHPantom does: analysing a large Laravel project asks it millions of times, over only a few thousand distinct classes. Every question used to hash the name and take a read lock on two shared indexes, so with one worker per CPU core the workers spent much of their time queueing behind each other instead of analysing. Each worker now remembers the answers it has already looked up and drops them the moment the class indexes change, so a repeated question costs a single pointer comparison. Whole-project analysis is 8-12% faster on large Laravel projects and uses up to a third less CPU, with identical results and no measurable change in memory use. Hover, completion, and go-to-definition resolve names through the same path and get the same saving.
 - **Vendor package scanning no longer reads every file twice.** Startup used to scan each vendor file once to find its classes, functions, and constants, then read and scan it again just to classify which package it came from for completion ranking. Each file's package is now known before it is scanned, so both are done in a single parallel pass, roughly halving the I/O and CPU cost of the vendor scan.
+- **Whole-workspace class pre-resolution now spreads across cores.** Resolving every known class in dependency order after indexing ran on a single core, leaving a single-threaded pause between the parallel index and the parallel diagnostic pass that grew with the number of classes in the project. That work is now shared across a pool of workers, which cuts the phase to roughly a quarter of its previous time on large Laravel projects and takes close to 20% off whole-project analysis on the largest of them. The editor's post-startup pre-resolution goes through the same path, so a large project becomes fully warm sooner. Results are unchanged.
 
 ### Removed
 

docs/todo.md

@@ -199,6 +199,7 @@ unlikely to move the needle for most users.
 | P46 | [`mago-phpdoc-syntax` cannot parse `@method static (…) name()`](todo/performance.md#p46-mago-phpdoc-syntax-cannot-parse-method-static--name)                                | Low         | Low         |
 | P30 | [Evaluate migrating parse/resolve/docblock pipeline to `mago-hir`](todo/performance.md#p30-evaluate-migrating-parseresolvedocblock-pipeline-to-mago-hir) (parked — re-evaluated at mago 1.45.0, still no `mago-hir` consumers upstream) | Medium-High | High        |
 | P43 | [`init_single_project` is the longest single-threaded stretch of a run](todo/performance.md#p43-init_single_project-is-the-longest-single-threaded-stretch-of-a-run)        | Medium-High | Medium      |
+| P47 | [The resolved-class cache lock caps concurrent class resolution](todo/performance.md#p47-the-resolved-class-cache-lock-caps-concurrent-class-resolution)                     | Medium      | Medium-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      |

docs/todo/performance.md

@@ -816,36 +816,91 @@ waiting on the triggers above.
 
 ---
 
-## P34. Eager class population is single-threaded
-
-**Impact: Medium · Effort: Medium**
-
-`populate_from_sorted` (`src/virtual_members/resolve.rs`) resolves the
-toposorted class list in a serial loop on one dedicated thread, in both
-the `analyze` Phase 1.5 and the LSP server's post-index
-`eager_populate_resolved_classes`. On a large Laravel project this is
-~1 s of single-core work in a release build and scales linearly with
-class count; on very large workspaces it is a visible single-threaded
-stretch between the (parallel) index and the (parallel) diagnostics
-pass.
-
-Resolution is already safe to run concurrently: the cycle-break
-in-flight set is keyed by `(thread, FQN)`, and two threads resolving
-the same class merely duplicate work, with one result winning the
-cache write. Two directions:
-
-1. **Work-stealing over the sorted list.** N workers pull indices from
-   an atomic counter over the existing toposorted vector. Most
-   dependencies are already cached by the time a dependent is pulled;
-   occasional duplicated resolution near list neighbours is bounded and
-   correct. Smallest change.
-2. **Topological generations.** Have the toposort emit levels
-   (classes whose dependencies are all in earlier levels) and fan each
-   level out across a scoped thread pool with a barrier between levels.
-   No duplicated work, slightly more coordination.
-
-Both need the workers on `PARSE_WORKER_STACK_SIZE` stacks, matching the
-current single populate thread.
+## P47. The resolved-class cache lock caps concurrent class resolution
+
+**Impact: Medium · Effort: Medium-High**
+
+`ResolvedClassCache` is a single `RwLock<ResolvedCacheInner>`, and every
+resolution takes its write lock several times: twice per
+`resolve_class_fully_inner` call to mark and clear the cycle-break
+in-flight marker, once to insert the finished class, and once per
+transformed member that `intern_transformed_method` /
+`intern_transformed_property` has to build (the read side of interning
+takes the read lock even on a hit). Member interning dominates the
+volume by far, since a merge produces one lookup per inherited or
+synthesized member.
+
+Measured with the eager-population worker pool
+(`populate_from_sorted`) swept from 1 to 32 workers on a 16-core / 32-thread
+SMT machine (Ryzen 9 5950X), release build, large Laravel projects: wall
+time falls steeply to ~8 workers, flattens through 16, and then
+*regresses* past that. On one project 2.1k classes went 0.62 s
+(1 worker) → 0.16 s (8) → 0.27 s (32), i.e. 32 workers were worse than
+4. Duplicated resolution is not the cause: the count of full resolutions
+rises only 5.9 % from 1 to 32 workers.
+
+Two confounds on this hardware make the raw sweep numbers hard to read
+at face value. The 16→32 leg is SMT: past 16 threads two workers share
+a physical core's execution resources rather than getting one each,
+which degrades throughput on its own and independently amplifies any
+lock contention (a spinning waiter now trashes the lock-holder's cache
+lines on the same core instead of a separate one). More importantly,
+the 5950X is dual-CCD — cores 0-7 and 8-15 (and their SMT siblings
+16-23/24-31) sit behind two separate L3 caches (`lscpu -e=CPU,CORE,CACHE`
+shows the split), and cache-line traffic for anything shared, including
+this lock, crosses the Infinity Fabric between them at much higher
+latency than a same-CCD hop. An unpinned process asking for ≤8 threads
+tends to get packed onto one CCD by the scheduler; past 8 it necessarily
+spills onto the second. Confirmed directly with `taskset`: 8 workers
+pinned to `0-7` (one CCD, no SMT) averaged 1.79 s wall for the same
+whole-project run that split 4+4 across both CCDs (`0,1,2,3,8,9,10,11`,
+still 8 distinct physical cores, still no SMT) averaged 2.02-2.17 s —
+15-20 % slower from CCD-crossing alone, with identical core count and
+no hyperthreading involved. So the knee at 8 in the original unpinned
+sweep is at least partly a topology artifact of this machine, not
+solely "how much lock contention exists at that many workers" — the
+same experiment on a single-CCD or single-die machine would likely
+plateau at a different worker count. `MAX_POPULATE_WORKERS` is pinned
+to 8 regardless: it is the largest value that stayed reliably within
+one CCD's worth of cores in testing, so it also happens to dodge the
+cross-CCD lock-traffic penalty as a side effect, not just diminishing
+per-lock-acquisition returns.
+
+Implication for the fix: fixing the lock removes the *within-CCD*
+contention this section measured, and should let population usefully
+raise `MAX_POPULATE_WORKERS` above 8. But raising it past one CCD's
+core count re-introduces cross-CCD traffic for whatever of the lock
+(sharded or not) is still shared — a plain `Vec<Mutex<Shard>>` does not
+avoid that on its own. Re-run the `taskset` same-CCD-vs-split comparison
+above after any lock-splitting change before raising the cap past 8, to
+check how much of the remaining ceiling is the lock versus the CCD
+boundary.
+
+The same lock is on the diagnostic pass's hot path (see P35), so
+splitting it should also help there. Directions, cheapest first:
+
+1. **Take the in-flight set off the shared lock.** It is already keyed
+   `(ThreadId, FQN)` purely to emulate a thread-local, so making it an
+   actual thread-local `HashSet<Atom>` removes two write locks per
+   resolution with no semantic change. Measured on its own this did
+   *not* move the sweep, so it is a prerequisite rather than the fix,
+   but it is nearly free.
+2. **Shard the interning tables.** `substituted_methods` /
+   `substituted_properties` are keyed by origin pointer and a
+   fingerprint hash, so they shard cleanly (e.g. by low bits of the
+   key) into independent locks without changing what gets shared. This
+   is where the volume is.
+3. **Separate the interning tables from the class map entirely.** The
+   two have different access patterns (interning is
+   write-heavy-then-read-heavy per merge; the class map is one insert
+   per class) and only share a lock for convenience. Note the member
+   sharing they provide is load-bearing for memory, so any change must
+   keep cross-class sharing intact rather than falling back to
+   per-thread tables.
+
+Before implementing, confirm the attribution by counting lock
+acquisitions per site during a population run: the sweep above proves
+*a* lock is the ceiling but not which acquirer dominates.
 
 ---
 

src/analyse/run.rs

@@ -202,27 +202,14 @@ pub async fn run(options: AnalyseOptions) -> i32 {
         let uri_classes_index = backend.symbols.uri_classes_index.read();
         crate::toposort::toposort_from_uri_classes_index(&uri_classes_index)
     };
-    // Run on a dedicated large-stack thread: `resolve_class_fully_inner`
-    // can nest deeply when the toposort misses dependencies (stubs,
-    // dynamically loaded classes), and this runs on a Tokio worker whose
-    // stack is the 2 MB default rather than the main thread's 8 MB.
-    std::thread::scope(|s| {
-        let backend = &backend;
-        let sorted_fqns = &sorted_fqns;
-        std::thread::Builder::new()
-            .name("eager-populate".into())
-            .stack_size(crate::PARSE_WORKER_STACK_SIZE)
-            .spawn_scoped(s, move || {
-                let class_loader =
-                    |name: &str| -> Option<Arc<ClassInfo>> { backend.find_or_load_class(name) };
-                crate::virtual_members::populate_from_sorted(
-                    sorted_fqns,
-                    &backend.resolved_class_cache,
-                    &class_loader,
-                );
-            })
-            .expect("failed to spawn eager-population thread");
-    });
+    // `populate_from_sorted` fans the list out over its own large-stack
+    // workers, so this needs no wrapper thread of its own.
+    let class_loader = |name: &str| -> Option<Arc<ClassInfo>> { backend.find_or_load_class(name) };
+    crate::virtual_members::populate_from_sorted(
+        &sorted_fqns,
+        &backend.resolved_class_cache,
+        &class_loader,
+    );
     // ── Phase 2: Collect diagnostics (parallel) ─────────────────────
     // Call individual collectors directly (instead of the grouped
     // collect_slow_diagnostics) so we can time each one independently.

src/backend/eager_population.rs

@@ -43,25 +43,15 @@ impl Backend {
                 let uri_classes_index = backend.symbols.uri_classes_index.read();
                 crate::toposort::toposort_from_uri_classes_index(&uri_classes_index)
             };
-            // Dedicated large-stack thread: class resolution walks
-            // parsed ASTs and can nest when the toposort misses
-            // dependencies (stubs, on-demand vendor loads).
-            std::thread::scope(|s| {
-                let backend = &backend;
-                let sorted_fqns = &sorted_fqns;
-                std::thread::Builder::new()
-                    .name("eager-populate".into())
-                    .stack_size(crate::PARSE_WORKER_STACK_SIZE)
-                    .spawn_scoped(s, move || {
-                        let class_loader = |name: &str| backend.find_or_load_class(name);
-                        crate::virtual_members::populate_from_sorted(
-                            sorted_fqns,
-                            &backend.resolved_class_cache,
-                            &class_loader,
-                        );
-                    })
-                    .expect("failed to spawn eager-populate thread");
-            });
+            // `populate_from_sorted` fans the list out over its own
+            // large-stack workers, so this needs no wrapper thread of
+            // its own.
+            let class_loader = |name: &str| backend.find_or_load_class(name);
+            crate::virtual_members::populate_from_sorted(
+                &sorted_fqns,
+                &backend.resolved_class_cache,
+                &class_loader,
+            );
         })
         .await;
     }