Laravel string keys are collected once instead of once per CPU core

Author: AJenbo

docs/CHANGELOG.md

@@ -48,6 +48,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 - **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.
 - **Lower memory use for member access spans.** The subject text recorded for every `->`/`::` access (e.g. `$this` in `$this->save()`) no longer allocates a string when it is a plain slice of the source, which covers the vast majority of accesses in typical PHP code. It now reuses the file's own bytes instead, with an allocation only for the rarer cases (chained calls, `new` expressions) where the recorded text differs from the source.
 - **Faster diagnostics on large projects.** Several diagnostic checks (by-reference parameter detection, the `Stringable`-to-`string` acceptance check, and `model-property<Model>` literal validation) now read a class's inheritance from the resolved-class cache instead of re-merging traits, parent classes, and generics from scratch on every call. On large Laravel projects this removed a measurable share of the diagnostic pass's CPU time, and as a side effect these checks now also see interface-declared members (e.g. a `__toString` declared only on an implemented interface).
+- **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.
 
 ### Removed
 

docs/todo.md

@@ -197,6 +197,7 @@ unlikely to move the needle for most users.
 |     | **[Performance](todo/performance.md)**                                                                                       |             |             |
 | 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        |
+| 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      |
 | 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

@@ -894,39 +894,60 @@ current single populate thread.
 **Impact: Medium-High · Effort: Medium**
 
 Measured on a 32-core machine against large Laravel projects (release
-build): the `analyze` Phase 2 diagnostic pass peaks around 14 cores and
-averages fewer, despite spawning one worker per core with atomic
-work-stealing. The original worst offender (every worker deep-cloning
+build): the `analyze` Phase 2 diagnostic pass spawns one worker per
+core with atomic work-stealing but does not keep them busy. Two
+offenders are fixed. The original one was every worker deep-cloning
 the embedded stub class/function/constant indexes twice per file via
-`clone_for_diagnostic_worker`) is fixed; the indexes are Arc-shared
-now. The remaining ceiling, per a frame-pointer `perf` profile of the
-diag workers after that fix:
-
-- ~18% of worker on-CPU time is still malloc/free/memmove, now diffuse:
+`clone_for_diagnostic_worker`; the indexes are Arc-shared now. The
+second was the Laravel string-key enumerations
+(`cached_route_names`/`cached_config_keys`/`cached_view_names`/
+`cached_trans_keys`/`cached_config_trees`): each walks the workspace
+from disk, and the plain check-then-fill cache stampeded, so all 32
+workers missed the same empty slot at once and each repeated the same
+gitignore-aware walk. They are guarded by
+`LaravelStringKeyBuildLocks` now, which took Phase 2 on a large
+Laravel project from 11.6 to 22.6 of 32 cores (1.91 s → 1.31 s) and
+whole-run wall clock down ~15%.
+
+Sampling `/proc/<pid>/task/*/stat` is the fastest way to see the
+remaining ceiling: workers in `S` rather than `R` are blocked, not
+computing. After the stampede fix the remaining gap is:
+
+- ~18% of worker on-CPU time is malloc/free/memmove, diffuse:
   `PhpType`, `MethodInfo`, and `ParameterInfo` clones throughout the
-  type engine, plus glibc malloc arena contention (visible as
-  `arena_get2`/futex kernel time) when 32 workers allocate heavily.
-- Read-lock contention on `fqn_class_index` and `class_not_found_cache`
-  is measurable but small (~1.5% on-CPU in the rwlock slow paths).
+  type engine, plus allocator arena contention when 32 workers
+  allocate heavily.
+- The `PhpType` interner (`php_type::intern`) is hit ~13 M times in a
+  1.3 s Phase 2, roughly a quarter of which fall through the shard
+  read lock to the write path. Its 64 shards are the largest single
+  source of measured lock-wait time left (~5 s of thread-time across
+  all workers), and `sched_yield` volume shows the workers spinning
+  before parking.
+- `ensure_workspace_indexed_with_progress` still re-walks the whole
+  workspace on every call, so the four surviving string-key
+  enumerations do four full walks per diagnostic pass (down from ~44).
+  The walk is deliberate — it is how PHP files created outside the
+  editor get discovered — so removing it needs a change to that
+  contract, not just another cache.
 - Projects that keep substantial code in `vendor/` (e.g. models shipped
   in a shared vendor package) pay extra: eager population only walks
   classes from the indexed user files, so every vendor class is parsed
   lazily on first touch during the diagnostic pass, serialising workers
   on the load path early in the run. Measured on a large Laravel
   project (release build, 32 threads): eager population covers only
-  ~1,300 user classes, and the diagnostic pass then resolves ~17,400
-  vendor classes lazily (~19 s of thread-CPU, roughly 30% of the
-  run's total CPU) with
-  6,425 cycle-break re-merges that eager dependency-first ordering
-  would have avoided (eager population itself hit only 3).
+  ~1,300 user classes, and the diagnostic pass then resolves ~2,100
+  further classes lazily, with cycle-break re-merges that eager
+  dependency-first ordering would have avoided (eager population
+  itself hit only 3).
 
 Likely next steps, in rough value order: reduce clone traffic in the
 hot type-engine paths (share via `Arc`/`Cow` instead of cloning
-`PhpType`/member vectors), include depended-upon vendor classes in
-eager population, and only then consider allocator-level fixes. The
-LSP workspace diagnostics pass uses the same collectors and has the
-same ceiling. Re-measure with `perf` (frame-pointer build) or the
-CPU-sampling loop in the Appendix after any change.
+`PhpType`/member vectors), cut interner traffic or widen its sharding,
+include depended-upon vendor classes in eager population, and only
+then consider allocator-level fixes. The LSP workspace diagnostics
+pass uses the same collectors and has the same ceiling. Re-measure
+with `perf` (frame-pointer build) or the CPU-sampling loop in the
+Appendix after any change.
 
 ---
 
@@ -952,6 +973,38 @@ headless mode rather than special-casing the `analyze` call site.
 
 ---
 
+## P43. `init_single_project` is the longest single-threaded stretch of a run
+
+**Impact: Medium-High · Effort: Medium**
+
+With the diagnostic pass now reaching ~22 of 32 cores (P35), project
+init is the least parallel phase left and the largest remaining share
+of wall clock. Per-phase timing of `analyze` on a large Laravel
+project (32-core machine, release build, warm page cache):
+
+| Phase | Wall | Avg cores |
+| --- | --- | --- |
+| `init_single_project` | 1.09 s | 2.3 |
+| `discover_user_files` | 0.03 s | 0.8 |
+| Phase 1 index | 0.10 s | 7.1 |
+| Phase 1.5 eager populate | 0.22 s | 1.0 |
+| Phase 2 diagnostics | 1.31 s | 22.6 |
+
+Init is ~40% of the run at barely two cores. It covers composer
+reading, autoload/classmap scanning, stub setup, and the vendor
+package scan, and it gates everything after it, so the same stretch is
+in front of the LSP's time-to-usable as well as the CLI's. Worth a
+per-phase breakdown inside init before choosing a fix — P32 (vendor
+scan reads every file twice) and P34 (eager population is
+single-threaded, the 1.0-core row above) are both already-filed pieces
+of the same window.
+
+Reproduce with the CPU-sampling loop in the Appendix, or by reading
+`utime + stime` from `/proc/self/stat` at each phase boundary and
+dividing by the phase's wall time.
+
+---
+
 # Remaining anti-pattern fixes
 
 Most remaining depth-cap issues were addressed by eager class

src/completion/laravel_string_keys.rs

@@ -394,52 +394,68 @@ impl Backend {
         keys
     }
 
-    pub(crate) fn cached_route_names(&self) -> Vec<String> {
-        {
-            let cache = self.laravel_string_key_cache.read();
-            if let Some(ref names) = cache.route_names {
-                return names.clone();
-            }
+    /// Read one slot of [`LaravelStringKeyCache`], building it under
+    /// `build_lock` when empty.
+    ///
+    /// The build is guarded rather than raced because every enumeration
+    /// walks the workspace from disk: the parallel diagnostic pass
+    /// otherwise has all N workers miss the same empty slot at once and
+    /// each repeat the identical walk. Waiters re-check the slot after
+    /// acquiring the guard, so exactly one walk happens per
+    /// invalidation.
+    pub(crate) fn cached_laravel_enumeration<T: Clone>(
+        &self,
+        build_lock: &parking_lot::Mutex<()>,
+        read: impl Fn(&crate::LaravelStringKeyCache) -> Option<T>,
+        store: impl Fn(&mut crate::LaravelStringKeyCache, T),
+        build: impl FnOnce() -> T,
+    ) -> T {
+        if let Some(cached) = read(&self.laravel_string_key_cache.read()) {
+            return cached;
         }
-        let names = crate::virtual_members::laravel::enumerate_all_route_names(self);
-        self.laravel_string_key_cache.write().route_names = Some(names.clone());
-        names
+        let _build_guard = build_lock.lock();
+        if let Some(cached) = read(&self.laravel_string_key_cache.read()) {
+            return cached;
+        }
+        let value = build();
+        store(&mut self.laravel_string_key_cache.write(), value.clone());
+        value
+    }
+
+    pub(crate) fn cached_route_names(&self) -> Vec<String> {
+        self.cached_laravel_enumeration(
+            &self.laravel_string_key_build_locks.route_names,
+            |cache| cache.route_names.clone(),
+            |cache, names| cache.route_names = Some(names),
+            || crate::virtual_members::laravel::enumerate_all_route_names(self),
+        )
     }
 
     pub(crate) fn cached_config_keys(&self) -> Vec<String> {
-        {
-            let cache = self.laravel_string_key_cache.read();
-            if let Some(ref keys) = cache.config_keys {
-                return keys.clone();
-            }
-        }
-        let keys = self.enumerate_all_config_keys();
-        self.laravel_string_key_cache.write().config_keys = Some(keys.clone());
-        keys
+        self.cached_laravel_enumeration(
+            &self.laravel_string_key_build_locks.config_keys,
+            |cache| cache.config_keys.clone(),
+            |cache, keys| cache.config_keys = Some(keys),
+            || self.enumerate_all_config_keys(),
+        )
     }
 
     pub(crate) fn cached_view_names(&self) -> Vec<String> {
-        {
-            let cache = self.laravel_string_key_cache.read();
-            if let Some(ref names) = cache.view_names {
-                return names.clone();
-            }
-        }
-        let names = self.enumerate_all_view_names();
-        self.laravel_string_key_cache.write().view_names = Some(names.clone());
-        names
+        self.cached_laravel_enumeration(
+            &self.laravel_string_key_build_locks.view_names,
+            |cache| cache.view_names.clone(),
+            |cache, names| cache.view_names = Some(names),
+            || self.enumerate_all_view_names(),
+        )
     }
 
     pub(crate) fn cached_trans_keys(&self) -> Vec<String> {
-        {
-            let cache = self.laravel_string_key_cache.read();
-            if let Some(ref keys) = cache.trans_keys {
-                return keys.clone();
-            }
-        }
-        let keys = self.enumerate_all_trans_keys();
-        self.laravel_string_key_cache.write().trans_keys = Some(keys.clone());
-        keys
+        self.cached_laravel_enumeration(
+            &self.laravel_string_key_build_locks.trans_keys,
+            |cache| cache.trans_keys.clone(),
+            |cache, keys| cache.trans_keys = Some(keys),
+            || self.enumerate_all_trans_keys(),
+        )
     }
 }
 
@@ -995,4 +1011,50 @@ final class UserPermissionController extends BaseController\n\
             );
         }
     }
+
+    /// Concurrent first callers must share one build, not run one each:
+    /// every enumeration behind these accessors walks the workspace from
+    /// disk, and the diagnostic pass hits them from all N workers at once.
+    #[test]
+    fn concurrent_first_callers_build_the_enumeration_once() {
+        use std::sync::atomic::{AtomicUsize, Ordering};
+
+        let backend = crate::Backend::new_test();
+        let builds = AtomicUsize::new(0);
+        let build_lock = parking_lot::Mutex::new(());
+
+        let results: Vec<Vec<String>> = std::thread::scope(|scope| {
+            let handles: Vec<_> = (0..16)
+                .map(|_| {
+                    let backend = &backend;
+                    let builds = &builds;
+                    let build_lock = &build_lock;
+                    scope.spawn(move || {
+                        backend.cached_laravel_enumeration(
+                            build_lock,
+                            |cache| cache.route_names.clone(),
+                            |cache, names| cache.route_names = Some(names),
+                            || {
+                                builds.fetch_add(1, Ordering::SeqCst);
+                                // Long enough that an unguarded
+                                // check-then-fill has every thread miss.
+                                std::thread::sleep(std::time::Duration::from_millis(50));
+                                vec!["home".to_string()]
+                            },
+                        )
+                    })
+                })
+                .collect();
+            handles.into_iter().map(|h| h.join().unwrap()).collect()
+        });
+
+        assert_eq!(
+            builds.load(Ordering::SeqCst),
+            1,
+            "the enumeration must be built once and shared, not once per caller"
+        );
+        for names in &results {
+            assert_eq!(names, &vec!["home".to_string()]);
+        }
+    }
 }

src/lib.rs

@@ -332,6 +332,28 @@ pub(crate) struct LaravelStringKeyCache {
     >,
 }
 
+/// Compute-once guards for the entries of [`LaravelStringKeyCache`].
+///
+/// Every enumeration behind that cache walks the workspace from disk.
+/// A plain check-then-fill cache stampedes under the parallel
+/// diagnostic pass: all workers miss the empty slot in the same
+/// instant, and each one repeats the identical walk while the others
+/// queue behind the workspace index lock. Holding the matching guard
+/// across the build means one worker walks and the rest wait once,
+/// then read the filled slot.
+///
+/// One guard per slot rather than one shared guard: the enumerations
+/// are independent, and a shared guard would let a worker that needs
+/// view names wait out an unrelated route-name build.
+#[derive(Default)]
+pub(crate) struct LaravelStringKeyBuildLocks {
+    pub route_names: parking_lot::Mutex<()>,
+    pub config_keys: parking_lot::Mutex<()>,
+    pub view_names: parking_lot::Mutex<()>,
+    pub trans_keys: parking_lot::Mutex<()>,
+    pub config_trees: parking_lot::Mutex<()>,
+}
+
 impl LaravelStringKeyCache {
     fn invalidate_for_uri(&mut self, uri: &str) {
         if uri.contains("/routes/") {
@@ -596,6 +618,9 @@ pub struct Backend {
     /// or `lang/` is updated.
     pub(crate) laravel_provider_resources: Arc<RwLock<virtual_members::laravel::ProviderResources>>,
     pub(crate) laravel_string_key_cache: Arc<RwLock<LaravelStringKeyCache>>,
+    /// Compute-once guards for `laravel_string_key_cache`; see
+    /// [`LaravelStringKeyBuildLocks`].
+    pub(crate) laravel_string_key_build_locks: Arc<LaravelStringKeyBuildLocks>,
     pub(crate) schema_index: Arc<RwLock<virtual_members::laravel::database_schema::SchemaIndex>>,
     /// Per-target member completion cache.
     ///
@@ -864,6 +889,7 @@ impl Backend {
                 virtual_members::laravel::ProviderResources::default(),
             )),
             laravel_string_key_cache: Arc::new(RwLock::new(LaravelStringKeyCache::default())),
+            laravel_string_key_build_locks: Arc::new(LaravelStringKeyBuildLocks::default()),
             schema_index: Arc::new(RwLock::new(
                 virtual_members::laravel::database_schema::SchemaIndex::default(),
             )),
@@ -948,6 +974,7 @@ impl Backend {
                 virtual_members::laravel::ProviderResources::default(),
             )),
             laravel_string_key_cache: Arc::new(RwLock::new(LaravelStringKeyCache::default())),
+            laravel_string_key_build_locks: Arc::new(LaravelStringKeyBuildLocks::default()),
             schema_index: Arc::new(RwLock::new(
                 virtual_members::laravel::database_schema::SchemaIndex::default(),
             )),
@@ -1556,6 +1583,7 @@ impl Backend {
             laravel_date_seed_uris: Arc::clone(&self.laravel_date_seed_uris),
             laravel_provider_resources: Arc::clone(&self.laravel_provider_resources),
             laravel_string_key_cache: Arc::clone(&self.laravel_string_key_cache),
+            laravel_string_key_build_locks: Arc::clone(&self.laravel_string_key_build_locks),
             schema_index: Arc::clone(&self.schema_index),
             member_completion_cache: Arc::clone(&self.member_completion_cache),
             stub_function_index: Arc::clone(&self.stub_function_index),

src/virtual_members/laravel/config_values.rs

@@ -485,15 +485,12 @@ impl Backend {
     }
 
     pub(crate) fn cached_config_trees(&self) -> Vec<(String, ConfigNode)> {
-        {
-            let cache = self.laravel_string_key_cache.read();
-            if let Some(ref trees) = cache.config_trees {
-                return trees.clone();
-            }
-        }
-        let trees = self.enumerate_config_trees();
-        self.laravel_string_key_cache.write().config_trees = Some(trees.clone());
-        trees
+        self.cached_laravel_enumeration(
+            &self.laravel_string_key_build_locks.config_trees,
+            |cache| cache.config_trees.clone(),
+            |cache, trees| cache.config_trees = Some(trees),
+            || self.enumerate_config_trees(),
+        )
     }
 
     fn enumerate_config_trees(&self) -> Vec<(String, ConfigNode)> {