Skip to content

Commit 8f3cda6

Browse files
committed
Saving a file no longer re-analyses every other open tab
1 parent e722043 commit 8f3cda6

8 files changed

Lines changed: 72 additions & 52 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
6060
- **Argument checking no longer slows down quadratically with file size.** The argument-count and argument-type checks know the byte offset of every call they inspect, but used to convert it into an editor line/column position and immediately back again before looking up the called function. Each of those conversions re-read the file from the beginning to count characters, so the cost of checking one call grew with the size of the file around it and a file with thousands of calls spent nearly all its time on offset arithmetic. The offset is now used directly. On a 370 KB file containing 2200 calls the two checks together drop from 13.7 seconds to 0.2, taking the whole file from 16.7 seconds to 3.4, and analysing the project it belongs to is close to three times faster.
6161
- **Faster workspace symbol search.** Matching a symbol against the "Go to Symbol in Workspace" query no longer allocates a lowercased copy of every class, method, property, constant, and function name in the project on each keystroke; matching is now done byte-wise in place for the common case of ASCII identifiers.
6262
- **Faster Eloquent scope-method resolution.** Injecting a model's scope methods onto its Builder used to re-walk the model's full inheritance chain (traits and parent classes) from scratch on every `Builder<Model>` instantiation. That base resolution is now cached, so a file with many instantiations of the same model's Builder resolves its scopes once instead of repeatedly.
63+
- **Saving a file no longer re-analyses every other open tab.** A save used to re-run the full diagnostic pass, the most expensive thing PHPantom does, on all open files in case any of them depended on the saved one. It now works out what the save actually changed (a class, one of its members, a function, a constant) and re-analyses only the open files that mention one of those names. Editing a method body reaches only the files that use the class, and renaming or retyping a member reaches only the files that use that member; unrelated tabs are left alone, so completion and hover stay responsive right after a save. Cases the comparison cannot narrow, such as saving a Laravel config, translation, or route file, still refresh every open file as before.
6364

6465
### Removed
6566

docs/todo.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,7 +199,6 @@ unlikely to move the needle for most users.
199199
| 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 |
200200
| 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 |
201201
| P16 | [Pre-parsed stub format (eliminate raw PHP embedding)](todo/performance.md#p16-pre-parsed-stub-format-eliminate-raw-php-embedding) | High | Medium-High |
202-
| 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 |
203202
| P3 | Parallel pre-filter in `find_implementors` | Low-Medium | Medium |
204203
| P6 | O(n²) transitive eviction in `evict_fqn` | Low | Low |
205204
| P15 | [Two-phase stub index construction (eliminate `RwLock` on stub maps)](todo/performance.md#p15-two-phase-stub-index-construction-eliminate-rwlock-on-stub-maps) | Low | Medium |

docs/todo/performance.md

Lines changed: 0 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -497,44 +497,6 @@ diagnostic layer.
497497

498498
---
499499

500-
## P22. Signature change re-queues slow diagnostics for every open file
501-
502-
**Impact: Medium-High · Effort: Medium**
503-
504-
When a file is saved, `schedule_diagnostics_for_open_files`
505-
(`src/diagnostics/mod.rs`, called from `did_save` in
506-
`src/server.rs`) queues **all** open files (minus the saved one)
507-
for a full slow-diagnostic pass — unknown classes, unknown
508-
members, argument checks. The per-file cost of that pass is the
509-
most expensive thing the server does (see the Appendix: tens of
510-
seconds on pathological files, hundreds of ms on ordinary ones).
511-
512-
A user with 20 tabs open therefore pays 20 full-file analysis
513-
passes per save, regardless of whether those files reference the
514-
edited class at all. (This used to fire on every signature-changing
515-
edit; it is now save-gated, which caps the frequency but not the
516-
O(open files) fan-out.) During the burst the diagnostic worker
517-
saturates blocking threads that completion/hover also need.
518-
519-
### Fix
520-
521-
Queue only files that can observe the change. The resolved-class
522-
cache already maintains a dependency index for transitive
523-
eviction (`evict_fqn`), and ER4 tracks which members changed.
524-
Build a reverse map (FQN → open files that reference it) — either
525-
from each file's `resolved_names` (which byte-offset FQN lookups
526-
already exist for) or by recording, during each diagnostic pass,
527-
which FQNs the pass touched. On signature change, queue only the
528-
dependent files. Falls back to all-open-files when the dependency
529-
data is missing (e.g. right after startup).
530-
531-
Synergy: P21 (offset-shifting) reduces the cost of re-diagnosing
532-
the *edited* file; this item reduces the *count* of other files
533-
re-diagnosed. Together they make the slow pass proportional to
534-
the blast radius of an edit.
535-
536-
---
537-
538500
## P46. `mago-phpdoc-syntax` cannot parse `@method static (…) name()`
539501

540502
**Impact: Low · Effort: Low (upstream)**

src/diagnostics/mod.rs

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@
177177
mod argument_count;
178178
pub(crate) mod class_case_mismatch;
179179
pub(crate) mod class_name_mismatch;
180+
pub(crate) mod cross_file;
180181
mod deprecated;
181182
mod external;
182183
pub(crate) mod helpers;
@@ -882,31 +883,30 @@ impl Backend {
882883
self.schedule_mago_analyze(uri);
883884
}
884885

885-
/// Invalidate diagnostics for all open files after a cross-file change.
886+
/// Invalidate diagnostics for the open files a save can affect.
886887
///
887-
/// Called when a class signature changes in one file, because
888-
/// diagnostics in other open files (unknown member, unknown class,
889-
/// deprecated usage) may depend on the changed class. The edited
890-
/// file itself is excluded (it is already scheduled by the caller).
888+
/// Diagnostics in other open files (unknown member, unknown class,
889+
/// deprecated usage, argument checks) can depend on the saved file, so
890+
/// they have to be recomputed — but only for the files that reference
891+
/// something the save changed. [`open_files_affected_by_save`] works
892+
/// out which those are, and falls back to every open file when it
893+
/// cannot tell. The saved file itself is excluded (it is already
894+
/// scheduled by the caller).
891895
///
892-
/// **Push mode:** Queues all open files for the background worker.
896+
/// **Push mode:** Queues those files for the background worker.
893897
///
894898
/// **Pull mode:** Invalidates cached full diagnostics and sends
895899
/// `workspace/diagnostic/refresh` so the editor re-pulls.
900+
///
901+
/// [`open_files_affected_by_save`]: Backend::open_files_affected_by_save
896902
pub(crate) fn schedule_diagnostics_for_open_files(&self, exclude_uri: &str) {
897903
if !self.init_complete.load(Ordering::Acquire) {
898904
return;
899905
}
900906

901907
let pull_mode = self.supports_pull_diagnostics.load(Ordering::Acquire);
902908

903-
let uris: Vec<String> = self
904-
.open_files
905-
.read()
906-
.keys()
907-
.filter(|u| u.as_str() != exclude_uri)
908-
.cloned()
909-
.collect();
909+
let uris = self.open_files_affected_by_save(exclude_uri);
910910
if uris.is_empty() {
911911
return;
912912
}

src/diagnostics/state.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,10 @@ pub(crate) struct DiagnosticState {
3838
pub(crate) workspace_diags: Arc<Mutex<WorkspaceDiagnostics>>,
3939
/// Prevents duplicate background workspace diagnostics passes.
4040
pub(crate) workspace_diag_pass_started: Arc<AtomicBool>,
41+
/// What each open file declared when it was last opened or saved, used
42+
/// to work out which other open files a save can affect. See
43+
/// [`crate::diagnostics::cross_file`].
44+
pub(crate) decl_baselines: Arc<Mutex<crate::diagnostics::cross_file::DeclarationBaselines>>,
4145
}
4246

4347
impl DiagnosticState {
@@ -53,6 +57,7 @@ impl DiagnosticState {
5357
suppressed: Arc::new(Mutex::new(Vec::new())),
5458
workspace_diags: Arc::new(Mutex::new(WorkspaceDiagnostics::default())),
5559
workspace_diag_pass_started: Arc::new(AtomicBool::new(false)),
60+
decl_baselines: Arc::new(Mutex::new(HashMap::new())),
5661
}
5762
}
5863
}

src/reference_index.rs

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,35 @@ impl Backend {
121121
Some(uris)
122122
}
123123

124+
/// Narrow `uris` to the files that reference one of `keys`.
125+
///
126+
/// A file the index does not track at all is kept: the index skips
127+
/// vendor files and stubs, and only records a file once it has been
128+
/// parsed, so an absent entry means "references unknown" rather than
129+
/// "references nothing". Returns `false` without touching `uris` when
130+
/// the index cannot answer yet (the workspace is still being indexed).
131+
pub(crate) fn retain_reference_candidates(
132+
&self,
133+
keys: &[ReferenceIndexKey],
134+
uris: &mut Vec<String>,
135+
) -> bool {
136+
if !self.workspace_indexed.load(Ordering::Acquire) {
137+
return false;
138+
}
139+
140+
let index = self.reference_index.read();
141+
let mut candidates = HashSet::new();
142+
for key in keys {
143+
if let Some(entries) = index.get(key) {
144+
candidates.extend(entries.keys().map(Arc::as_ref));
145+
}
146+
}
147+
uris.retain(|uri| {
148+
candidates.contains(uri.as_str()) || !index.uri_keys.contains_key(uri.as_str())
149+
});
150+
true
151+
}
152+
124153
pub(crate) fn reindex_references_for_symbol_maps_batch(
125154
&self,
126155
items: Vec<(String, Arc<SymbolMap>)>,

src/server.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -617,6 +617,10 @@ impl LanguageServer for Backend {
617617
// Parse and update AST map, use map, and namespace map
618618
self.update_ast(&uri, &text);
619619

620+
// Baseline for the first save: without it, that save would have to
621+
// re-diagnose every open file to be safe.
622+
self.capture_declaration_baseline(&uri);
623+
620624
// Schedule diagnostics asynchronously so that the first-open
621625
// response is not blocked by lazy stub parsing (which can take
622626
// tens of seconds when many class references trigger cache-miss
@@ -750,6 +754,7 @@ impl LanguageServer for Backend {
750754

751755
self.open_files.write().remove(&uri);
752756
self.did_change_parse_locks.lock().remove(&uri);
757+
self.clear_declaration_baseline(&uri);
753758

754759
// Drop coalescing state for this file so the maps don't grow unbounded
755760
// across an editing session.

src/types/mod.rs

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2108,6 +2108,18 @@ impl ClassInfo {
21082108
/// - `class_docblock` (adding/removing `@method`/`@property` tags)
21092109
/// - `laravel` metadata (affects virtual member providers)
21102110
pub fn signature_eq(&self, other: &ClassInfo) -> bool {
2111+
self.class_level_signature_eq(other) && self.members_signature_eq(other)
2112+
}
2113+
2114+
/// The class-level half of [`signature_eq`](Self::signature_eq):
2115+
/// everything except the declared methods, properties, and constants.
2116+
///
2117+
/// A change here (a new parent, a different `@mixin`, an edited
2118+
/// class docblock, changed Laravel metadata) can alter *any* member's
2119+
/// resolution, so callers that want to know which members an edit
2120+
/// affected must treat the whole member surface as affected when this
2121+
/// returns `false`.
2122+
pub fn class_level_signature_eq(&self, other: &ClassInfo) -> bool {
21112123
// ── Class-level metadata ────────────────────────────────────
21122124
if self.kind != other.kind
21132125
|| self.name != other.name
@@ -2139,6 +2151,13 @@ impl ClassInfo {
21392151
return false;
21402152
}
21412153

2154+
true
2155+
}
2156+
2157+
/// The member half of [`signature_eq`](Self::signature_eq): the
2158+
/// declared methods, properties, and constants, compared as
2159+
/// name-keyed sets so reordering members in source is not a change.
2160+
pub fn members_signature_eq(&self, other: &ClassInfo) -> bool {
21422161
// ── Methods (compared as a name-keyed set) ──────────────────
21432162
if self.methods.len() != other.methods.len() {
21442163
return false;

0 commit comments

Comments
 (0)