Skip to content

Commit 40c8c50

Browse files
committed
Avoid stub duplication on ever thread
1 parent 45554d6 commit 40c8c50

3 files changed

Lines changed: 57 additions & 37 deletions

File tree

docs/CHANGELOG.md

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

4747
### Fixed
4848

49+
- **Diagnostic and request workers no longer copy the embedded stub indexes.** Every diagnostic pass and every hover, completion, or go-to-definition request cloned the full embedded stub class, function, and constant indexes (thousands of entries each) instead of sharing them. During workspace analysis this happened twice per file, and the resulting allocation churn serialised the parallel diagnostic workers in the memory allocator. The indexes are now shared, roughly halving `analyze` wall time on large Laravel projects and removing the same overhead from every editor request.
4950
- **Member resolution no longer degrades on deeply nested class dependencies.** Class resolution previously cut off at internal nesting limits, silently returning incomplete member sets (missing completions, spurious unknown-member diagnostics) on projects with deeply intertwined hierarchies and virtual members. Those limits are gone: only genuine dependency cycles, such as two Eloquent models whose relationships reference each other, fall back to a partial view, and they now do so deterministically instead of depending on which class happened to resolve first on a given thread.
5051
- **Completion works after closing a multi-line closure argument on the same line as the next chain operator.** Typing `->` immediately after a call like `->map(function (...) { ... })->` now resolves the full receiver instead of returning no member suggestions until the operator is moved to a new line. Contributed by @calebdw.
5152
- **Conditional return types recognize interpolated strings as strings.** A function with a conditional return type like `($key is string ? mixed : null)` now correctly resolves the `string` branch when called with an interpolated string argument (e.g. `config("{$prefix}.host")`). Previously the interpolated string was not recognized as a string literal, causing the return type to fall through to the else branch and resolve as `null`. Contributed by @calebdw.

docs/todo/performance.md

Lines changed: 38 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -421,6 +421,16 @@ perf report --stdio --no-children | head -80
421421

422422
# Flamegraph (requires the `flamegraph` crate or perf-tools):
423423
perf script | flamegraph > /tmp/phpantom.svg
424+
425+
# Instantaneous CPU utilisation over a run (% of one core, sampled
426+
# every 0.5 s), for machines where perf is unavailable
427+
# (kernel.perf_event_paranoid > 2):
428+
./target/release/phpantom_lsp analyze --project-root <dir> --no-colour \
429+
>/dev/null 2>&1 & PID=$!; prev=0
430+
while kill -0 $PID 2>/dev/null; do
431+
cur=$(awk '{print $14+$15}' /proc/$PID/stat 2>/dev/null) || break
432+
[ -n "$cur" ] && echo $(( (cur - prev) * 2 )); prev=$cur; sleep 0.5
433+
done
424434
```
425435

426436
### Pathological test file
@@ -964,27 +974,34 @@ current single populate thread.
964974

965975
**Impact: Medium-High · Effort: Medium**
966976

967-
Measured on a 32-core machine against a large Laravel project (release
968-
build): the `analyze` Phase 2 diagnostic pass runs at ~2 to ~13 cores,
969-
averaging roughly 4, despite spawning one worker per core with atomic
970-
work-stealing. Utilisation is lowest at the start of the pass and
971-
climbs as it progresses, which points at lock contention that eases as
972-
shared caches warm, rather than at work starvation. Likely suspects:
973-
`find_or_load_class` taking write locks while lazily parsing vendor
974-
files early in the pass, and write contention on the resolved-class
975-
cache and chain-resolution caches. The LSP workspace diagnostics pass
976-
uses the same collectors and likely has the same ceiling. Needs
977-
profiling to attribute the blocked time before picking a fix;
978-
re-measure after any change with the CPU-sampling loop in the Appendix.
979-
980-
Projects that keep substantial code in `vendor/` (e.g. models shipped
981-
in a shared vendor package that the application depends on) hit this
982-
harder: eager population only walks classes from the indexed user
983-
files, so every vendor class is parsed lazily on first touch during
984-
the diagnostic pass. The observed CPU shape on such a project is
985-
bursty (alternating stalls and full-core bursts) consistent with one
986-
worker holding the load lock while the rest wait, then all proceeding
987-
until the next unparsed vendor class.
977+
Measured on a 32-core machine against large Laravel projects (release
978+
build): the `analyze` Phase 2 diagnostic pass peaks around 14 cores and
979+
averages fewer, despite spawning one worker per core with atomic
980+
work-stealing. The original worst offender (every worker deep-cloning
981+
the embedded stub class/function/constant indexes twice per file via
982+
`clone_for_diagnostic_worker`) is fixed; the indexes are Arc-shared
983+
now. The remaining ceiling, per a frame-pointer `perf` profile of the
984+
diag workers after that fix:
985+
986+
- ~18% of worker on-CPU time is still malloc/free/memmove, now diffuse:
987+
`PhpType`, `MethodInfo`, and `ParameterInfo` clones throughout the
988+
type engine, plus glibc malloc arena contention (visible as
989+
`arena_get2`/futex kernel time) when 32 workers allocate heavily.
990+
- Read-lock contention on `fqn_class_index` and `class_not_found_cache`
991+
is measurable but small (~1.5% on-CPU in the rwlock slow paths).
992+
- Projects that keep substantial code in `vendor/` (e.g. models shipped
993+
in a shared vendor package) pay extra: eager population only walks
994+
classes from the indexed user files, so every vendor class is parsed
995+
lazily on first touch during the diagnostic pass, serialising workers
996+
on the load path early in the run.
997+
998+
Likely next steps, in rough value order: reduce clone traffic in the
999+
hot type-engine paths (share via `Arc`/`Cow` instead of cloning
1000+
`PhpType`/member vectors), include depended-upon vendor classes in
1001+
eager population, and only then consider allocator-level fixes. The
1002+
LSP workspace diagnostics pass uses the same collectors and has the
1003+
same ceiling. Re-measure with `perf` (frame-pointer build) or the
1004+
CPU-sampling loop in the Appendix after any change.
9881005

9891006
---
9901007

src/lib.rs

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -491,7 +491,7 @@ pub struct Backend {
491491
/// Consulted by `find_or_load_class` as a final fallback after the
492492
/// `uri_classes_index` and PSR-4 resolution. Stub files are parsed lazily on
493493
/// first access and cached in `uri_classes_index` under `phpantom-stub://` URIs.
494-
pub(crate) stub_index: RwLock<CiMap<&'static str>>,
494+
pub(crate) stub_index: Arc<RwLock<CiMap<&'static str>>>,
495495
/// Cache of fully-resolved classes (inheritance + virtual members).
496496
///
497497
/// Keyed by fully-qualified class name. Populated lazily by
@@ -601,15 +601,15 @@ pub struct Backend {
601601
/// Filtered at startup via [`set_php_version`](Self::set_php_version) to
602602
/// remove stubs that do not exist in the target PHP version.
603603
/// Can be consulted to resolve return types of built-in function calls.
604-
pub(crate) stub_function_index: RwLock<CiMap<&'static str>>,
604+
pub(crate) stub_function_index: Arc<RwLock<CiMap<&'static str>>>,
605605
/// Embedded PHP stubs for built-in constants (e.g. `PHP_EOL`,
606606
/// `SORT_ASC`, …). Maps constant name → raw PHP source code.
607607
///
608608
/// Built once during construction via [`stubs::build_stub_constant_index`].
609609
/// Filtered at startup via [`set_php_version`](Self::set_php_version) to
610610
/// remove stubs that do not exist in the target PHP version.
611611
/// Can be consulted when resolving standalone constant references.
612-
pub(crate) stub_constant_index: RwLock<HashMap<&'static str, &'static str>>,
612+
pub(crate) stub_constant_index: Arc<RwLock<HashMap<&'static str, &'static str>>>,
613613
/// Diagnostic debouncing state and the pull-model diagnostic caches.
614614
pub(crate) diag: crate::diagnostics::state::DiagnosticState,
615615
/// PHPStan's dedicated background worker state (extremely slow and
@@ -815,9 +815,11 @@ impl Backend {
815815
phar_archives: Arc::new(RwLock::new(HashMap::new())),
816816
parsed_uris: Arc::new(RwLock::new(HashSet::new())),
817817
parse_inflight: Arc::new(resolution::ParseInflight::new()),
818-
stub_index: RwLock::new(CiMap::from(stubs::build_stub_class_index())),
819-
stub_function_index: RwLock::new(CiMap::from(stubs::build_stub_function_index())),
820-
stub_constant_index: RwLock::new(stubs::build_stub_constant_index()),
818+
stub_index: Arc::new(RwLock::new(CiMap::from(stubs::build_stub_class_index()))),
819+
stub_function_index: Arc::new(RwLock::new(CiMap::from(
820+
stubs::build_stub_function_index(),
821+
))),
822+
stub_constant_index: Arc::new(RwLock::new(stubs::build_stub_constant_index())),
821823
resolved_class_cache: virtual_members::new_resolved_class_cache(),
822824
auth_user_type_cache: Arc::new(RwLock::new(HashMap::new())),
823825
laravel_aliases: Arc::new(RwLock::new(None)),
@@ -898,9 +900,9 @@ impl Backend {
898900
phar_archives: Arc::new(RwLock::new(HashMap::new())),
899901
parsed_uris: Arc::new(RwLock::new(HashSet::new())),
900902
parse_inflight: Arc::new(resolution::ParseInflight::new()),
901-
stub_index: RwLock::new(CiMap::new()),
902-
stub_function_index: RwLock::new(CiMap::new()),
903-
stub_constant_index: RwLock::new(HashMap::new()),
903+
stub_index: Arc::new(RwLock::new(CiMap::new())),
904+
stub_function_index: Arc::new(RwLock::new(CiMap::new())),
905+
stub_constant_index: Arc::new(RwLock::new(HashMap::new())),
904906
resolved_class_cache: virtual_members::new_resolved_class_cache(),
905907
auth_user_type_cache: Arc::new(RwLock::new(HashMap::new())),
906908
laravel_aliases: Arc::new(RwLock::new(None)),
@@ -1004,7 +1006,7 @@ impl Backend {
10041006
pub fn new_test_with_stubs(stub_index: HashMap<&'static str, &'static str>) -> Self {
10051007
virtual_members::phpdoc::clear_mixin_cache();
10061008
let backend = Self {
1007-
stub_index: RwLock::new(CiMap::from(stub_index)),
1009+
stub_index: Arc::new(RwLock::new(CiMap::from(stub_index))),
10081010
..Self::test_defaults()
10091011
};
10101012
backend.set_php_version(backend.php_version());
@@ -1023,9 +1025,9 @@ impl Backend {
10231025
) -> Self {
10241026
virtual_members::phpdoc::clear_mixin_cache();
10251027
let backend = Self {
1026-
stub_index: RwLock::new(CiMap::from(stub_index)),
1027-
stub_function_index: RwLock::new(CiMap::from(stub_function_index)),
1028-
stub_constant_index: RwLock::new(stub_constant_index),
1028+
stub_index: Arc::new(RwLock::new(CiMap::from(stub_index))),
1029+
stub_function_index: Arc::new(RwLock::new(CiMap::from(stub_function_index))),
1030+
stub_constant_index: Arc::new(RwLock::new(stub_constant_index)),
10291031
..Self::test_defaults()
10301032
};
10311033
backend.set_php_version(backend.php_version());
@@ -1515,7 +1517,7 @@ impl Backend {
15151517
phar_archives: Arc::clone(&self.phar_archives),
15161518
parsed_uris: Arc::clone(&self.parsed_uris),
15171519
parse_inflight: Arc::clone(&self.parse_inflight),
1518-
stub_index: RwLock::new(self.stub_index.read().clone()),
1520+
stub_index: Arc::clone(&self.stub_index),
15191521
resolved_class_cache: Arc::clone(&self.resolved_class_cache),
15201522
auth_user_type_cache: Arc::clone(&self.auth_user_type_cache),
15211523
laravel_aliases: Arc::clone(&self.laravel_aliases),
@@ -1534,8 +1536,8 @@ impl Backend {
15341536
laravel_string_key_cache: Arc::clone(&self.laravel_string_key_cache),
15351537
schema_index: Arc::clone(&self.schema_index),
15361538
member_completion_cache: Arc::clone(&self.member_completion_cache),
1537-
stub_function_index: RwLock::new(self.stub_function_index.read().clone()),
1538-
stub_constant_index: RwLock::new(self.stub_constant_index.read().clone()),
1539+
stub_function_index: Arc::clone(&self.stub_function_index),
1540+
stub_constant_index: Arc::clone(&self.stub_constant_index),
15391541
diag: self.diag.clone(),
15401542
workspace: self.workspace.clone(),
15411543
phpstan_tool: self.phpstan_tool.clone(),

0 commit comments

Comments
 (0)