Skip to content

Commit 3095978

Browse files
committed
Find References, Rename, and Go to Implementation no longer look stalled
during startup
1 parent fc5026d commit 3095978

7 files changed

Lines changed: 146 additions & 273 deletions

File tree

docs/CHANGELOG.md

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

5555
### Fixed
5656

57+
- **Find References, Rename, and Go to Implementation no longer look stalled during startup indexing.** A search started while the background index is still parsing the workspace waits for that index to finish, since acting on a partial index would silently miss results. That wait now shows in the request's own progress bar as "Waiting for workspace index" alongside the index's live file counts, instead of sitting at "Resolving…" with no indication of what it is waiting for.
5758
- **Type narrowing against `@phpstan-assert`/`@psalm-assert` no longer leaks memory.** Evaluating a narrowing call such as `Assert::isInstanceOf($x, Foo::class)` or a custom function/method with the same annotations allocated a small amount of memory that was never freed. This ran on every conditional touched during completion, hover, diagnostics, and go-to-definition, so memory held by a long-running editor session grew slowly but permanently the more the project was edited. Fixed by no longer leaking the allocation.
5859
- **Renaming a namespace no longer corrupts group `use` statements.** Renaming a namespace segment that is imported with a group `use` (e.g. `use App\Old\{Foo, Bar};`) previously rewrote the group's shared prefix and then also spliced the new prefix into each member name, producing invalid PHP like `use App\New\{App\New\Foo, Bar};`. The member names are left untouched now, since the prefix rewrite alone already updates the whole statement correctly.
5960
- **Parameter name inlay hints no longer shift to the wrong parameter when only part of a multi-line call is visible.** Editors request inlay hints only for the currently visible viewport, and when a call's arguments were split across the viewport boundary, every hint after the first excluded argument was labelled with the previous parameter's name instead of its own. This was most noticeable on multi-line constructor calls with several arguments, such as those using constructor property promotion.

docs/todo.md

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,9 +25,6 @@ within the same impact tier.
2525

2626
| # | Item | Impact | Effort |
2727
| --- | --------------------------------------------------------------------------------------------------------------------- | ---------- | ------ |
28-
| P36 | [Diagnostic-path call sites re-merge inheritance per call](todo/performance.md#p36-diagnostic-path-call-sites-re-merge-inheritance-per-call-instead-of-reading-the-resolved-class-cache) | Medium | Low |
29-
| 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 |
30-
| 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 |
3128
| 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 |
3229
| L18 | [`Macroable::mixin()` registrations](todo/laravel.md) | Low | Medium |
3330
| L1 | [Facade completion](todo/laravel.md#l1-facade-completion-upstream-method-template-improvement) | High | High |

docs/todo/indexing.md

Lines changed: 0 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -343,50 +343,6 @@ ready to implement.
343343

344344
---
345345

346-
## X10. Interactive requests block on the workspace index lock during initial indexing
347-
348-
**Impact: Medium · Effort: Medium**
349-
350-
The full background index holds `workspace_index_lock` for its entire
351-
run. Any request that reaches `ensure_workspace_indexed` during that
352-
window parks on the lock until the whole workspace parse finishes
353-
(tens of seconds on large projects):
354-
355-
- **Laravel string-key completion** (`config('`, `route('`, `view('`,
356-
`__('`): the key enumerations call `user_file_symbol_maps()`, so
357-
the first such completion during startup stalls instead of
358-
returning what is already parsed. Same for the invalid-string-key
359-
diagnostics pass on open files.
360-
- **Find References / Rename**: these need complete data, so waiting
361-
is semantically right, but the user gets no signal that the wait is
362-
the index (the request's own progress token stays at "Resolving…").
363-
- **Go to Implementation** (full strategy, index not yet ready):
364-
blocks the same way.
365-
366-
Core completion, hover, and go-to-definition are unaffected (they use
367-
lazy per-class loading and never touch the index lock), which is the
368-
main responsiveness guarantee to preserve.
369-
370-
### Direction
371-
372-
Completeness-critical consumers (references, rename, GTI) should keep
373-
waiting but report "waiting for workspace index" through their
374-
progress token. Best-effort consumers (string-key completion, the
375-
string-key diagnostics pass) should not call
376-
`ensure_workspace_indexed` while `full_index_in_progress` is set;
377-
they should serve partial results from the current `symbol_maps`
378-
snapshot and rely on the post-index `workspace/diagnostic/refresh`
379-
to correct anything that was missing. Additionally, the config /
380-
view / translation enumerations are directory-scoped (`config/`,
381-
`resources/views/`, `lang/`) and could be fed by targeted scans that
382-
do not depend on the full workspace parse at all.
383-
384-
Related but separate: the deferred X2 priority-aware scheduling
385-
covers the CPU-contention side (index workers saturating all cores);
386-
this item covers the lock-blocking side.
387-
388-
---
389-
390346
## X9. Honor editor file excludes and PHP associations during indexing
391347

392348
**Impact: Low-Medium · Effort: Medium**

docs/todo/performance.md

Lines changed: 0 additions & 207 deletions
Original file line numberDiff line numberDiff line change
@@ -856,71 +856,6 @@ and delete the second pass entirely.
856856

857857
---
858858

859-
## P33. Workspace diagnostics leaves the whole project fully resolved in memory
860-
861-
**Impact: High · Effort: Medium-High**
862-
863-
Enabling background workspace diagnostics raises resident memory
864-
several-fold over a full index alone. Measured on a mid-size Laravel
865-
project (~1,200 indexed user files, ~3,000 resolved classes) the live
866-
set after the pass is ~1.16 GB, of which the resolved-class cache is
867-
**~748 MB — about 65%**. Ranked by the RSS each structure releases
868-
when dropped: resolved-class cache ~748 MB, class indexes
869-
(`method_store` + `fqn_class_index` + `uri_classes_index`) ~99 MB,
870-
`symbol_maps` ~26 MB, `reference_index` ~14 MB. The cache
871-
holds ~247 KB per resolved class.
872-
873-
Two compounding causes:
874-
875-
1. **Inheritance flattening stores a member slot per class.** Each
876-
fully-resolved class stores the complete flattened member set (own +
877-
trait + parent + interface + mixin), so a project with ~3 K classes
878-
at ~214 members each holds ~648 K method slots. The distinct
879-
*allocations* behind those slots are now heavily shared for methods,
880-
properties, and constants alike: a slot the merge does not rewrite
881-
points back at its source `Arc<MethodInfo>`/`Arc<PropertyInfo>`/
882-
`Arc<ConstantInfo>`, and the copies a merge does produce (template
883-
substitution, trait / interface / enrichment merges, Eloquent Builder
884-
and scope forwarding) are interned so value-identical results
885-
collapse to one allocation with a shared parameter list. What remains
886-
per class is the flattened `Vec<Arc<_>>` itself plus its
887-
`method_index` — one pointer slot per visible member. Measured on a
888-
full Laravel app (~2 K resolved classes) this residual is only
889-
~10 MB across the whole resolved-class cache; the member
890-
*allocations* the vecs point at, not the pointer slots, are the bulk.
891-
Bounding the cache (fix below) reclaims both; separating the pointer
892-
slots on their own is not worth the rewrite.
893-
894-
2. **The cache is retained for the whole session, unbounded.** Eager
895-
population plus lazy resolution during the pass fill the cache with
896-
every class in the project, and nothing evicts it afterwards.
897-
Interactive requests only ever touch a small working set, so the bulk
898-
sits resident for nothing. (Disabling eager population does not help:
899-
the per-file diagnostic pass refills the cache to the same size.)
900-
901-
Fix:
902-
903-
1. **Bound the resolved-class cache.** After the workspace pass, the
904-
fully-flattened metadata for every project class does not need to stay
905-
resident. Either drop the cache when the pass finishes (interactive
906-
requests re-resolve lazily) or make it an LRU with a size cap. The
907-
recursion-guard elimination made a wholesale clear safe: resolution
908-
cycles are broken by the cache's in-flight set, which `clear()`
909-
deliberately leaves untouched, so clearing mid-resolution cannot
910-
resurrect unbounded recursion. **Low-Medium.**
911-
912-
Separating the per-class member vectors from a global member store
913-
(Mago's model, resolving members on demand instead of flattening a
914-
`Vec` of pointers) was the other candidate here, but measurement showed
915-
the residual pointer slots + `method_index` are only ~10 MB across the
916-
whole cache once member allocations are shared and interned. That is not
917-
worth moving substitution to query time (the transform-on-inherit model
918-
means an inherited member is not byte-identical to the declaring class's
919-
once template substitution or bare-`self` rewriting applies), so the
920-
lever is bounding the cache, not separating the metadata.
921-
922-
---
923-
924859
## P34. Eager class population is single-threaded
925860

926861
**Impact: Medium · Effort: Medium**
@@ -995,148 +930,6 @@ CPU-sampling loop in the Appendix after any change.
995930

996931
---
997932

998-
## P36. Diagnostic-path call sites re-merge inheritance per call instead of reading the resolved-class cache
999-
1000-
**Impact: Medium · Effort: Low**
1001-
1002-
Six call sites on the diagnostic path call
1003-
`resolve_class_with_inheritance` directly, re-running the full
1004-
inheritance merge (traits, parent chain, generic substitution) on every
1005-
call and discarding the result after reading a single member:
1006-
1007-
- `src/diagnostics/undefined_variables/mod.rs` — by-reference parameter
1008-
positions for static, constructor, and instance calls (3 sites).
1009-
- `src/diagnostics/type_errors/compatibility.rs` — the
1010-
Stringable/`__toString` acceptance check and the
1011-
`model-property<Model>` literal check (2 sites).
1012-
- `src/class_lookup.rs` — the `model-property<Model>` literal subtype
1013-
check (1 site).
1014-
1015-
Measured on a large Laravel project (release build, 32 threads): 25,676
1016-
direct merge calls costing ~7 s of thread-CPU during the diagnostic
1017-
pass, roughly 11% of the run's total CPU — while a resolved-class cache
1018-
hit serves the same class in ~3 µs. The Stringable check is the likely
1019-
hot one (it fires for every object-argument-to-string-parameter
1020-
comparison).
1021-
1022-
Route these through `resolve_class_fully_maybe_cached`; the
1023-
resolved-class cache is active during diagnostic passes
1024-
(`with_active_resolved_class_cache`), and `undefined_variables` has
1025-
`self` (the `Backend`) in scope already. Note the cached result
1026-
additionally includes interface members and virtual members. For these
1027-
member-existence and parameter lookups that is a strict improvement
1028-
(e.g. a `__toString` declared on an interface is currently missed), but
1029-
verify the affected diagnostics against the fixture suites for
1030-
unexpected acceptance changes.
1031-
1032-
The provider-internal callers in `src/virtual_members/` and the
1033-
interface merge inside `resolve_class_fully_inner` are intentional
1034-
(they run *while* the cache entry is being built) and are out of scope.
1035-
1036-
**Still worth doing, and orthogonal to the memory work below.** This is
1037-
a CPU fix (11% of diagnostic-pass CPU), not a memory one, but it is
1038-
memory-neutral-to-positive: today each of the 25,676 calls builds a
1039-
full inheritance-merged `ClassInfo` (its own methods/properties vecs)
1040-
and immediately discards it — 25,676 transient allocations that
1041-
straight-line CPU savings would remove as a side effect. Routing
1042-
through the cache turns most of those into a hit against an entry the
1043-
diagnostic pass populates anyway; it can only add a new persistent
1044-
cache entry for a class that was otherwise *never* resolved in the
1045-
pass, which is a narrow case bounded by the same fix as P33.
1046-
1047-
---
1048-
1049-
## Memory baseline (2026-07-28, refreshed after `PhpType` interning)
1050-
1051-
Measured with the `mem-audit` cargo feature, which installs a counting
1052-
global allocator and prints a deep-size walk of every long-lived store
1053-
at the end of `analyze` when `PHPANTOM_MEM_AUDIT` is set:
1054-
1055-
```sh
1056-
cargo build --release --features mem-audit
1057-
PHPANTOM_MEM_AUDIT=1 ./target/release/phpantom_lsp analyze \
1058-
--project-root <dir> --no-colour
1059-
```
1060-
1061-
The walk dedups shared `Arc`s by pointer, so a member reached from
1062-
several classes is counted once. It reports both a field-level
1063-
attribution and a drop-and-measure probe that clears each store and
1064-
reads the exact allocator delta, which cross-checks the walk. Since
1065-
which allocator is active changes the RSS/overhead figures, the audit
1066-
labels its own build (`target … / … libc, allocator …`) —
1067-
always check that line before comparing a new run to the table below.
1068-
1069-
The byte/allocation figures (live heap, per-store bytes released,
1070-
`PhpType` duplication) are allocator-independent — they come from
1071-
`size_of`/`capacity()`, not from the allocator — and were cross-checked
1072-
identical under both `System` and `mimalloc`. Only RSS and the
1073-
"overhead" line are allocator-specific; those are reported here for
1074-
mimalloc, since that is what every shipped Linux binary runs (see
1075-
`docs/BUILDING.md` for reproducing that exact build locally):
1076-
1077-
| Measure | Mid-size Laravel app | Large Laravel app |
1078-
| ----------------------------- | -------------------- | ----------------- |
1079-
| Peak RSS (`VmHWM`) | 402 MB | 540 MB |
1080-
| RSS after mimalloc collect | 370 MB | 510 MB |
1081-
| Live heap (allocator-reported)| 245 MB | 358 MB |
1082-
| Live allocations | 1.44 M | 2.21 M |
1083-
| Allocator + page overhead | 125 MB (34% of RSS) | 152 MB (30%) |
1084-
1085-
The glibc figures below were taken against the original baseline (the
1086-
one with a 64-byte `PhpType`, before it was shrunk and then interned)
1087-
and have not been re-measured since; the relative picture they
1088-
illustrate is unchanged. Under plain glibc
1089-
`malloc` (no mimalloc) the same run peaked higher despite a *lower*
1090-
overhead percentage: 622 MB / 855 MB peak, 100 MB (19%) / 133 MB (17%)
1091-
overhead. mimalloc holds more
1092-
allocator/page overhead in percentage terms but a smaller absolute
1093-
peak, because PHPantom already tunes it (`configure_allocator`: a
1094-
short purge delay plus a per-process THP opt-out) to return freed
1095-
parse-burst memory promptly.
1096-
1097-
The overhead percentage rose as later changes (P33 sharing, P38
1098-
interning) cut the live-allocation count — evidence that this overhead
1099-
tracks the size of mimalloc's retained free-page pool, not a flat
1100-
per-allocation cost, and that pool is sized by the parse-burst peak
1101-
rather than the steady-state live set. Reducing allocation count on
1102-
its own is therefore not expected to move it further; a real
1103-
reduction would have to target that peak instead.
1104-
1105-
Bytes released per store (drop-and-measure, mid-size / large):
1106-
1107-
| Store | Released | Allocations |
1108-
| ---------------------- | ---------------- | --------------- |
1109-
| `resolved_class_cache` | 101 MB / 165 MB | 719 K / 1.18 M |
1110-
| `symbol_maps` | 30 MB / 47 MB | 159 K / 248 K |
1111-
| `uri_classes_index` | 16 MB / 23 MB | 104 K / 149 K |
1112-
| `reference_index` | 5.3 MB / 8.1 MB | 73 K / 112 K |
1113-
| `resolved_names` | 4.6 MB / 6.7 MB | 53 K / 75 K |
1114-
| everything else | < 6 MB each ||
1115-
1116-
Within the member data, the largest field-level groups on the large app
1117-
are `MethodInfo` structs (78 MB across 232 K distinct methods at 336 B
1118-
each), parameter vectors (36 MB across 191 K vectors holding 325 K
1119-
parameters at 88 B each), member docblock text (13 MB), and
1120-
`method_index` (12 MB). Type payloads no longer register: interning
1121-
collapsed 856 K live type occurrences on that app to 51 K distinct
1122-
nodes, so the whole type representation now costs under 4 MB.
1123-
1124-
Two findings from the baseline are worth recording because they close
1125-
off obvious-looking ideas:
1126-
1127-
- **Boxing the display-only docblock fields is a net loss.** Only 9-10%
1128-
of methods have no description, `@return` text, links, `@see` refs, or
1129-
deprecation data, so moving the six fields behind one `Option<Box<…>>`
1130-
would add a pointer plus an allocation header to the other 90%. The
1131-
same holds for constants. Properties and parameters are mostly
1132-
doc-free but their doc fields are few, so the win there is ~2-8 MB and
1133-
not worth the indirection on its own.
1134-
- **A shared empty `SharedVec` saves under 1 MB.** Only 13% of parameter
1135-
vectors are empty. Worth folding into another change that touches
1136-
`SharedVec`, not worth a commit by itself.
1137-
1138-
---
1139-
1140933
## P42. Reference index is built during the headless `analyze` pipeline but never queried there
1141934

1142935
**Impact: Low-Medium · Effort: Low**

0 commit comments

Comments
 (0)