Skip to content

Commit d482e8a

Browse files
committed
Add performance to roadmap
1 parent 558e2f4 commit d482e8a

3 files changed

Lines changed: 105 additions & 6 deletions

File tree

docs/todo.md

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ with each step.
2828
| [Configuration](todo/config.md) | Per-project `.phpantom.toml` file, PHP version override, diagnostic tool toggles, prompt-and-remember settings |
2929
| [Refactoring](todo/refactor.md) | Technical debt and cleanup tasks. Gate check between sprints: clear all items before starting the next sprint |
3030
| [Indexing](todo/indexing.md) | Self-generated classmap, staleness detection, parallel file processing, full background indexing, disk cache |
31+
| [Performance](todo/performance.md) | FQN index, `Arc<ClassInfo>`, `RwLock`, inheritance dedup, file content cloning, type substitution optimisation |
3132

3233
---
3334

@@ -56,6 +57,49 @@ requirement to a performance tip instead of a prerequisite.
5657

5758
---
5859

60+
## Sprint 2.5 — Performance foundations
61+
62+
These structural fixes reduce latency and contention on the hot paths
63+
that every LSP request exercises. They are sequenced here, before
64+
refactoring and feature sprints, because later work amplifies the
65+
underlying costs:
66+
67+
- **Parallel file processing** (indexing.md Phase 3) adds concurrent
68+
readers. Without `RwLock`, parallelism is defeated by lock
69+
contention.
70+
- **Full background indexing** (indexing.md Phase 5) puts thousands of
71+
files into `ast_map`. Without the FQN index, every class lookup
72+
becomes an O(thousands) scan. Without `Arc<ClassInfo>`, every lookup
73+
deep-clones a large struct.
74+
- **Sprint 6 Laravel generics** (`collect()`, custom builders) increase
75+
the depth of generic substitution chains. Without the `HashSet` dedup
76+
and substitution early-exit, Eloquent models pay O(N²) per resolution.
77+
78+
Fixing these first means every subsequent sprint benefits automatically.
79+
80+
| # | Item | Effort | Domain | Doc Link |
81+
|---|---|---|---|---|
82+
| 82 | FQN secondary index for `find_class_in_ast_map` | Low | Performance | [performance.md §1](todo/performance.md#1-fqn-secondary-index-for-find_class_in_ast_map) |
83+
| 83 | `RwLock` for read-heavy maps | Low | Performance | [performance.md §3](todo/performance.md#3-rwlock-for-read-heavy-maps) |
84+
| 84 | `HashSet` dedup in inheritance merging | Low | Performance | [performance.md §4](todo/performance.md#4-hashset-dedup-in-inheritance-merging) |
85+
| 85 | `Arc<String>` for file content in `open_files` | Low | Performance | [performance.md §5](todo/performance.md#5-arcstring-for-file-content-in-open_files) |
86+
| 86 | `Arc<SymbolMap>` to avoid snapshot cloning | Low | Performance | [performance.md §6](todo/performance.md#6-arcsymbolmap-to-avoid-snapshot-cloning) |
87+
| 87 | Reference-counted `ClassInfo` (`Arc<ClassInfo>`) | Medium | Performance | [performance.md §2](todo/performance.md#2-reference-counted-classinfo-arcclassinfo) |
88+
| 88 | Early-exit and `Cow` return in `apply_substitution` | Low | Performance | [performance.md §7](todo/performance.md#7-recursive-string-substitution-in-apply_substitution) |
89+
90+
Item 89 (incremental text sync) is in the backlog because the parser
91+
requires a full re-parse regardless, limiting the benefit to IPC
92+
bandwidth savings on large files.
93+
94+
**After Sprint 2.5:** Every resolution path is O(1) lookup instead of
95+
O(N) scan. Concurrent reads no longer block each other. Inheritance
96+
merging is O(N) instead of O(N²). File content and symbol maps are
97+
shared by reference instead of deep-cloned. The codebase is ready for
98+
parallel file processing and full background indexing without
99+
performance regressions.
100+
101+
---
102+
59103
## Sprint 3 — Refactoring
60104

61105
Extract Function is the remaining refactoring pillar. Inline Variable,
@@ -116,7 +160,7 @@ deepens that lead and rounds out the remaining feature surface.
116160
| 39 | Simplify with null coalescing / null-safe operator (code action) | Medium | Code Actions | [actions.md §2](todo/actions.md#2-simplify-with-null-coalescing--null-safe-operator) |
117161
| 40 | Inlay hints (`textDocument/inlayHint`) | Medium | LSP Features | [lsp-features.md §9](todo/lsp-features.md#9-inlay-hints-textdocumentinlayhint) |
118162

119-
**After Sprint 6:** PHPantom has a complete, polished LSP feature set.
163+
**After Sprint 5:** PHPantom has a complete, polished LSP feature set.
120164
Users moving to Zed/Neovim/Helix lose nothing on the intelligence side
121165
and gain 1000× faster startup. The remaining gaps are Blade and
122166
formatting (not our domain).
@@ -152,7 +196,7 @@ the Laravel-specific manifestation is a small incremental step. Item 51
152196
(Type Hierarchy) depends on the go-to-implementation infrastructure and
153197
should be scheduled after that work is stable. Item 56 (partial result
154198
streaming) addresses outbound latency for large result sets. See also
155-
item 73 (incremental text sync) in the backlog, which addresses the
199+
item 89 (incremental text sync) in the backlog, which addresses the
156200
complementary inbound direction.
157201

158202
---
@@ -206,8 +250,13 @@ eventually but don't move the needle.
206250
| # | Item | Effort | Domain | Doc Link |
207251
|---|---|---|---|---|
208252
| 72 | Switch → match conversion | Medium | Code Actions | [actions.md §4](todo/actions.md#4-switch--match-conversion) |
209-
| 73 | Incremental text sync | Medium | LSP Features | [lsp-features.md §17](todo/lsp-features.md#17-incremental-text-sync) |
253+
| 89 | Incremental text sync | Medium | Performance | [performance.md §8](todo/performance.md#8-incremental-text-sync) |
210254

255+
### Performance long-tail
256+
257+
| # | Item | Effort | Domain | Doc Link |
258+
|---|---|---|---|---|
259+
| 90 | Type AST for `apply_substitution` (full refactor) | High | Performance | [performance.md §7](todo/performance.md#7-recursive-string-substitution-in-apply_substitution) |
211260

212261
---
213262

@@ -242,5 +291,4 @@ Blade is a multi-phase project tracked in [todo/blade.md](todo/blade.md).
242291
| Phase 1 | Blade-to-PHP preprocessor | Module skeleton, directive translation, source map, LSP wiring |
243292
| Phase 2 | Component support | Template/component discovery, `<x-component>` parsing, `@props`/`@aware`, name completion |
244293
| Phase 3 | Cross-file view intelligence | View name GTD, signature merging for `@extends`, component→template variable typing |
245-
| Phase 4 | Blade directive completion | Directive name completion with snippet insertion |
246-
294+
| Phase 4 | Blade directive completion | Directive name completion with snippet insertion |

docs/todo/indexing.md

Lines changed: 47 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,24 @@ happens rarely (a few times per day at most).
194194
go-to-implementation, self-scan, diagnostics) by processing files in
195195
parallel with priority awareness.
196196

197+
**Prerequisites (from [performance.md](performance.md)):**
198+
199+
- **§3 `RwLock` for read-heavy maps.** Parallel `spawn_blocking` tasks
200+
need concurrent read access to `ast_map`, `symbol_maps`, `use_map`,
201+
and `namespace_map`. With `Mutex`, every reader serializes against
202+
every other reader, defeating the purpose of parallelism regardless
203+
of how clever the priority scheduling is. `RwLock` lets all readers
204+
proceed concurrently; only writes (which happen on `did_change` and
205+
on-demand file loading) take an exclusive lock.
206+
- **§5 `Arc<String>` for file content.** Parallel tasks that need file
207+
content should share references rather than each cloning the full
208+
string out of `open_files`.
209+
- **§6 `Arc<SymbolMap>` to avoid snapshot cloning.** Find References
210+
snapshots all user-file symbol maps before scanning. With owned
211+
`SymbolMap` values, this deep-clones hundreds of maps. With
212+
`Arc<SymbolMap>`, the snapshot is cheap and the lock is released
213+
immediately.
214+
197215
### Why not rayon?
198216

199217
`rayon` is the obvious choice for "process N files in parallel" and
@@ -289,6 +307,28 @@ is a nice-to-have, not a requirement.
289307
enabling workspace symbols, fast find-references without on-demand
290308
scanning, and complete completion item detail.
291309

310+
**Prerequisites (from [performance.md](performance.md)):**
311+
312+
- **§1 FQN secondary index.** The second pass calls `update_ast` on
313+
every file, populating `ast_map` with thousands of entries. Without
314+
a FQN index, every `find_class_in_ast_map` call (Phase 1 of
315+
`find_or_load_class`) becomes an O(thousands) linear scan. With the
316+
index, it is O(1).
317+
- **§2 `Arc<ClassInfo>`.** Full indexing stores a `ClassInfo` for every
318+
class in the project. Without `Arc`, every resolution clones the
319+
entire struct out of the map. With `Arc`, retrieval is a
320+
reference-count increment. This is the difference between full
321+
indexing using ~200 MB vs. ~500 MB for a large project.
322+
- **§3 `RwLock`.** The second pass writes to `ast_map` at Low priority
323+
while High-priority LSP requests read from it. `Mutex` would force
324+
every completion/hover request to wait for the current background
325+
parse to finish its map insertion. `RwLock` lets reads proceed
326+
concurrently with other reads; only the brief write window blocks.
327+
- **§4 `HashSet` dedup.** Full indexing means every class resolution
328+
pulls from a fully populated inheritance tree. Eloquent models with
329+
150+ inherited methods across 8+ levels hit the O(N²) dedup path
330+
on every resolution. The `HashSet` fix brings this to O(N).
331+
292332
### Trigger
293333

294334
When `strategy = "full"` is set in `.phpantom.toml`.
@@ -343,7 +383,13 @@ Currently we store `ClassInfo`, `FunctionInfo`, and `SymbolMap`
343383
structs that are not as lean as they could be. For a 21K-file
344384
codebase, full indexing will use meaningful RAM. This is acceptable
345385
because it's an opt-in mode, but we should profile and trim struct
346-
sizes over time. The aim is to stay under 512MB for a full project.
386+
sizes over time. The aim is to stay under 512 MB for a full project.
387+
388+
The performance prerequisites above (§2 `Arc<ClassInfo>`, §5
389+
`Arc<String>`, §6 `Arc<SymbolMap>`) directly reduce memory usage by
390+
sharing data across the ast_map, caches, and snapshot copies instead
391+
of deep-cloning each. These should be measured before and after to
392+
validate the 512 MB target.
347393

348394
### Workspace symbols
349395

docs/todo/lsp-features.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -525,6 +525,11 @@ acceptable.
525525
## 17. Incremental text sync
526526
**Impact: Low-Medium · Effort: Medium**
527527

528+
> **Cross-reference:** This item is also tracked as
529+
> [performance.md §8](performance.md#8-incremental-text-sync) and
530+
> roadmap item 89. The canonical spec lives here; the performance
531+
> document includes it for completeness.
532+
528533
PHPantom uses `TextDocumentSyncKind::FULL`, meaning every
529534
`textDocument/didChange` notification sends the entire file content.
530535
Switching to `TextDocumentSyncKind::INCREMENTAL` means the client sends

0 commit comments

Comments
 (0)