Skip to content

Commit 45554d6

Browse files
committed
Improve startup performance
1 parent ac3b3f6 commit 45554d6

3 files changed

Lines changed: 121 additions & 35 deletions

File tree

docs/CHANGELOG.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
3535

3636
### Changed
3737

38-
- **Classes are pre-resolved for the whole workspace after startup.** Once the background index completes, every known class is resolved in dependency order even when workspace diagnostics are disabled, so the first completion, hover, or go-to-definition against any class reads a warm cache instead of resolving on demand. Edits still re-resolve only the affected classes. Contributed by @AJenbo.
38+
- **Classes are pre-resolved for the whole workspace after startup.** Once the background index completes, every known class is resolved in dependency order even when workspace diagnostics are disabled, so the first completion, hover, or go-to-definition against any class reads a warm cache instead of resolving on demand. Edits still re-resolve only the affected classes.
3939
- **Continuous progress reporting.** The indexing progress bar now advances file by file with live counts (e.g. "Scanning vendor packages (3201/8544 files)") instead of jumping between a few fixed milestones. This covers single-project, monorepo, and non-Composer workspaces. Go to Implementation, Find References, and Type Hierarchy show the same live progress while they scan, including when one of them triggers the first full workspace index.
4040
- **Property hover now shows effective types as a `var` detail line.** Property hovers now mirror method hovers by displaying the resolved/effective property type above the PHP snippet as `**var**`, while the snippet itself shows only the native PHP property declaration. This keeps docblock-inferred, virtual, and schema-derived property types out of the generated signature block. Contributed by @calebdw.
4141
- **Updated the bundled mago toolchain to 1.43.0.** The parser, docblock parser, formatter, and supporting crates are refreshed to the latest upstream release. Contributed by @enwi in https://github.com/PHPantom-dev/phpantom_lsp/pull/234.
@@ -46,7 +46,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
4646

4747
### Fixed
4848

49-
- **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. Contributed by @AJenbo.
49+
- **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.
5050
- **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.
5151
- **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.
5252
- **Renaming a constructor-promoted property parameter now cascades to `$this->prop` usages.** Renaming `private int $someField` in a constructor's parameter list previously only updated the parameter declaration itself, leaving every `$this->someField` reference elsewhere in the class stale. Rename, find references, document highlight, and linked editing now treat a promoted property parameter the same as an ordinary property declaration.

docs/todo/performance.md

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -927,6 +927,67 @@ Fixes, cheapest first (each stands alone):
927927

928928
---
929929

930+
## P34. Eager class population is single-threaded
931+
932+
**Impact: Medium · Effort: Medium**
933+
934+
`populate_from_sorted` (`src/virtual_members/resolve.rs`) resolves the
935+
toposorted class list in a serial loop on one dedicated thread, in both
936+
the `analyze` Phase 1.5 and the LSP server's post-index
937+
`eager_populate_resolved_classes`. On a large Laravel project this is
938+
~1 s of single-core work in a release build and scales linearly with
939+
class count; on very large workspaces it is a visible single-threaded
940+
stretch between the (parallel) index and the (parallel) diagnostics
941+
pass.
942+
943+
Resolution is already safe to run concurrently: the cycle-break
944+
in-flight set is keyed by `(thread, FQN)`, and two threads resolving
945+
the same class merely duplicate work, with one result winning the
946+
cache write. Two directions:
947+
948+
1. **Work-stealing over the sorted list.** N workers pull indices from
949+
an atomic counter over the existing toposorted vector. Most
950+
dependencies are already cached by the time a dependent is pulled;
951+
occasional duplicated resolution near list neighbours is bounded and
952+
correct. Smallest change.
953+
2. **Topological generations.** Have the toposort emit levels
954+
(classes whose dependencies are all in earlier levels) and fan each
955+
level out across a scoped thread pool with a barrier between levels.
956+
No duplicated work, slightly more coordination.
957+
958+
Both need the workers on `PARSE_WORKER_STACK_SIZE` stacks, matching the
959+
current single populate thread.
960+
961+
---
962+
963+
## P35. Diagnostic passes reach only a fraction of available cores
964+
965+
**Impact: Medium-High · Effort: Medium**
966+
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.
988+
989+
---
990+
930991
# Remaining anti-pattern fixes
931992

932993
Most remaining depth-cap issues are addressed by ER5 (class

src/reference_index.rs

Lines changed: 58 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,36 @@ pub(crate) struct ReferenceIndexEntry {
4040
pub(crate) is_declaration: bool,
4141
}
4242

43-
pub(crate) type ReferenceIndex = Arc<RwLock<HashMap<ReferenceIndexKey, Vec<ReferenceIndexEntry>>>>;
43+
/// The candidate index plus a reverse map of which keys each URI has
44+
/// entries under.
45+
///
46+
/// The reverse map keeps per-file eviction proportional to that file's
47+
/// own contributions. Without it, evicting a URI means scanning every
48+
/// entry of every key in the index — an O(whole-index) pass under the
49+
/// write lock that serialises the parallel indexing workers (each file
50+
/// they publish evicts itself first) and grows with workspace size on
51+
/// every keystroke.
52+
#[derive(Default)]
53+
pub(crate) struct ReferenceIndexInner {
54+
by_key: HashMap<ReferenceIndexKey, Vec<ReferenceIndexEntry>>,
55+
uri_keys: HashMap<String, Vec<ReferenceIndexKey>>,
56+
}
57+
58+
impl ReferenceIndexInner {
59+
pub(crate) fn get(&self, key: &ReferenceIndexKey) -> Option<&Vec<ReferenceIndexEntry>> {
60+
self.by_key.get(key)
61+
}
62+
63+
#[cfg(test)]
64+
pub(crate) fn is_empty(&self) -> bool {
65+
self.by_key.is_empty()
66+
}
67+
}
68+
69+
pub(crate) type ReferenceIndex = Arc<RwLock<ReferenceIndexInner>>;
4470

4571
pub(crate) fn new_reference_index() -> ReferenceIndex {
46-
Arc::new(RwLock::new(HashMap::new()))
72+
Arc::new(RwLock::new(ReferenceIndexInner::default()))
4773
}
4874

4975
impl Backend {
@@ -103,12 +129,18 @@ impl Backend {
103129
.filter_map(|(idx, item)| keep[idx].then_some(item))
104130
.collect();
105131

106-
let batch_uris: HashSet<String> = rebuilt.iter().map(|(uri, _)| uri.clone()).collect();
107132
let mut index = self.reference_index.write();
108-
evict_reference_index_uris_locked(&mut index, &batch_uris);
109-
for (_uri, entries) in rebuilt {
133+
for (uri, _) in &rebuilt {
134+
evict_reference_index_uri_locked(&mut index, uri);
135+
}
136+
for (uri, entries) in rebuilt {
137+
let mut keys: HashSet<ReferenceIndexKey> = HashSet::new();
110138
for (key, entry) in entries {
111-
index.entry(key).or_default().push(entry);
139+
index.by_key.entry(key.clone()).or_default().push(entry);
140+
keys.insert(key);
141+
}
142+
if !keys.is_empty() {
143+
index.uri_keys.insert(uri, keys.into_iter().collect());
112144
}
113145
}
114146
}
@@ -272,28 +304,18 @@ impl Backend {
272304
}
273305
}
274306

275-
fn evict_reference_index_uri_locked(
276-
index: &mut HashMap<ReferenceIndexKey, Vec<ReferenceIndexEntry>>,
277-
uri: &str,
278-
) {
279-
index.retain(|_, entries| {
280-
entries.retain(|entry| entry.uri != uri);
281-
!entries.is_empty()
282-
});
283-
}
284-
285-
fn evict_reference_index_uris_locked(
286-
index: &mut HashMap<ReferenceIndexKey, Vec<ReferenceIndexEntry>>,
287-
uris: &HashSet<String>,
288-
) {
289-
if uris.is_empty() {
307+
fn evict_reference_index_uri_locked(index: &mut ReferenceIndexInner, uri: &str) {
308+
let Some(keys) = index.uri_keys.remove(uri) else {
290309
return;
310+
};
311+
for key in keys {
312+
if let Some(entries) = index.by_key.get_mut(&key) {
313+
entries.retain(|entry| entry.uri != uri);
314+
if entries.is_empty() {
315+
index.by_key.remove(&key);
316+
}
317+
}
291318
}
292-
293-
index.retain(|_, entries| {
294-
entries.retain(|entry| !uris.contains(entry.uri.as_str()));
295-
!entries.is_empty()
296-
});
297319
}
298320

299321
fn normalize_symbol_name(name: impl AsRef<str>) -> String {
@@ -574,20 +596,23 @@ mod tests {
574596
}
575597

576598
#[test]
577-
fn empty_batch_evict_and_zero_member_offset_are_noops() {
578-
let mut index = HashMap::new();
579-
index.insert(
580-
ReferenceIndexKey::Class("Foo".to_string()),
599+
fn evicting_unknown_uri_and_zero_member_offset_are_noops() {
600+
let mut index = ReferenceIndexInner::default();
601+
let key = ReferenceIndexKey::Class("Foo".to_string());
602+
let uri = "file:///project/src/Foo.php".to_string();
603+
index.by_key.insert(
604+
key.clone(),
581605
vec![ReferenceIndexEntry {
582-
uri: "file:///project/src/Foo.php".to_string(),
606+
uri: uri.clone(),
583607
start: 0,
584608
end: 3,
585609
is_declaration: true,
586610
}],
587611
);
612+
index.uri_keys.insert(uri, vec![key.clone()]);
588613

589-
evict_reference_index_uris_locked(&mut index, &HashSet::new());
590-
assert!(index.contains_key(&ReferenceIndexKey::Class("Foo".to_string())));
614+
evict_reference_index_uri_locked(&mut index, "file:///project/src/Other.php");
615+
assert!(index.by_key.contains_key(&key));
591616
assert_eq!(member_range(0, "name", true), None);
592617
}
593618

0 commit comments

Comments
 (0)