Skip to content

Commit 7ff492e

Browse files
committed
Vendor package scanning no longer reads every file twice
1 parent 249b443 commit 7ff492e

5 files changed

Lines changed: 69 additions & 112 deletions

File tree

docs/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
5353
- **Faster diagnostics on large projects.** Several diagnostic checks (by-reference parameter detection, the `Stringable`-to-`string` acceptance check, and `model-property<Model>` literal validation) now read a class's inheritance from the resolved-class cache instead of re-merging traits, parent classes, and generics from scratch on every call. On large Laravel projects this removed a measurable share of the diagnostic pass's CPU time, and as a side effect these checks now also see interface-declared members (e.g. a `__toString` declared only on an implemented interface).
5454
- **Laravel string keys are collected once instead of once per CPU core.** Checking a `route()`, `config()`, `view()`, or `__()` key needs the project's full list of valid keys, and building each list walks the project directory. The diagnostic pass runs one worker per core, so all of them used to reach the empty list at the same moment and every one repeated the same walk, leaving most cores waiting on the disk rather than analysing. Each list is now built once and shared, which roughly doubles the number of cores the diagnostic pass keeps busy and cuts whole-project analysis time by around 15% on large Laravel projects.
5555
- **Repeated class lookups are remembered instead of searched again.** Answering "which class does this name refer to?" is the single most frequent thing PHPantom does: analysing a large Laravel project asks it millions of times, over only a few thousand distinct classes. Every question used to hash the name and take a read lock on two shared indexes, so with one worker per CPU core the workers spent much of their time queueing behind each other instead of analysing. Each worker now remembers the answers it has already looked up and drops them the moment the class indexes change, so a repeated question costs a single pointer comparison. Whole-project analysis is 8-12% faster on large Laravel projects and uses up to a third less CPU, with identical results and no measurable change in memory use. Hover, completion, and go-to-definition resolve names through the same path and get the same saving.
56+
- **Vendor package scanning no longer reads every file twice.** Startup used to scan each vendor file once to find its classes, functions, and constants, then read and scan it again just to classify which package it came from for completion ranking. Each file's package is now known before it is scanned, so both are done in a single parallel pass, roughly halving the I/O and CPU cost of the vendor scan.
5657

5758
### Removed
5859

docs/todo.md

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,6 @@ unlikely to move the needle for most users.
209209
| 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 |
210210
| P17 | [`mago-names` resolution on the parse hot path](todo/performance.md#p17-mago-names-resolution-on-the-parse-hot-path) | Medium | Low |
211211
| P18 | [Subtype result caching](todo/performance.md#p18-subtype-result-caching) (per-request HashMap for hierarchy walks) | Medium | Low |
212-
| P32 | [Vendor package scan reads every file twice for origin classification](todo/performance.md#p32-vendor-package-scan-reads-every-file-twice-for-origin-classification) | Medium | Low-Medium |
213212
| P20 | [Content-hash gated resolution cache persistence](todo/performance.md#p20-content-hash-gated-resolution-cache-persistence) | Medium | Medium |
214213
| P21 | [Offset-shifting for cached diagnostics on partial edits](todo/performance.md#p21-offset-shifting-for-cached-diagnostics-on-partial-edits) | Medium | Medium |
215214
| P23 | [`workspace/symbol` lowercases every symbol name per request](todo/performance.md#p23-workspacesymbol-allocates-a-lowercase-copy-of-every-symbol-name-per-request) | Low-Medium | Low |

docs/todo/performance.md

Lines changed: 3 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -816,30 +816,6 @@ waiting on the triggers above.
816816

817817
---
818818

819-
## P32. Vendor package scan reads every file twice for origin classification
820-
821-
**Impact: Medium · Effort: Low-Medium**
822-
823-
`scan_vendor_packages_with_skip` (`src/classmap_scanner.rs`) scans the
824-
collected vendor files twice on every startup:
825-
826-
1. `scan_files_parallel_full(&all_files, …)` reads and byte-scans every
827-
file in parallel to build the classmap/function/constant indices.
828-
2. A sequential origin-classification pass then **re-reads and
829-
re-scans the same files**`read_for_scan` + `scan_content` per
830-
PSR-4 file, and a full `scan_file_full` per plain file — solely to
831-
map each discovered symbol to its package's completion-origin tier.
832-
833-
That doubles the I/O and scan cost of the vendor scan, and the second
834-
pass is single-threaded. The origin is known per *file* before either
835-
pass runs (it comes from the package entry the file was collected
836-
under), so the classification can be produced during the parallel scan:
837-
thread the per-file origin through `scan_files_parallel_full` (e.g.
838-
scan `(PathBuf, origin)` pairs and emit origins alongside symbols)
839-
and delete the second pass entirely.
840-
841-
---
842-
843819
## P34. Eager class population is single-threaded
844820

845821
**Impact: Medium · Effort: Medium**
@@ -1061,10 +1037,9 @@ Init is ~40% of the run at barely two cores. It covers composer
10611037
reading, autoload/classmap scanning, stub setup, and the vendor
10621038
package scan, and it gates everything after it, so the same stretch is
10631039
in front of the LSP's time-to-usable as well as the CLI's. Worth a
1064-
per-phase breakdown inside init before choosing a fix — P32 (vendor
1065-
scan reads every file twice) and P34 (eager population is
1066-
single-threaded, the 1.0-core row above) are both already-filed pieces
1067-
of the same window.
1040+
per-phase breakdown inside init before choosing a fix — P34 (eager
1041+
population is single-threaded, the 1.0-core row above) is an
1042+
already-filed piece of the same window.
10681043

10691044
Reproduce with the CPU-sampling loop in the Appendix, or by reading
10701045
`utime + stime` from `/proc/self/stat` at each phase boundary and

src/classmap_scanner/discovery.rs

Lines changed: 65 additions & 81 deletions
Original file line numberDiff line numberDiff line change
@@ -403,48 +403,20 @@ pub fn scan_vendor_packages_with_skip(
403403
}
404404
}
405405

406-
// Phase 2: scan all collected files in parallel
407-
let mut all_files: Vec<PathBuf> = psr4_files.iter().map(|(path, _, _)| path.clone()).collect();
408-
all_files.extend(plain_files.iter().map(|(path, _)| path.clone()));
409-
410-
// The origin classification pass below re-reads every file, so it
411-
// counts as its own work units.
412-
progress_add_total(
413-
progress,
414-
all_files.len() + psr4_files.len() + plain_files.len(),
415-
);
416-
417-
let mut result = scan_files_parallel_full(&all_files, progress);
418-
let mut class_origins = HashMap::new();
419-
let mut function_origins = HashMap::new();
420-
let mut constant_origins = HashMap::new();
421-
for (path, expected_fqn, origin) in psr4_files {
422-
progress_add_done(progress);
423-
if let Ok(content) = read_for_scan(&path) {
424-
for fqn in scan_content(&content) {
425-
if fqn == expected_fqn {
426-
class_origins.entry(fqn).or_insert(origin);
427-
}
428-
}
429-
}
430-
}
431-
for (path, origin) in plain_files {
432-
progress_add_done(progress);
433-
let symbols = super::scan_file_full(&path);
434-
for fqn in symbols.classes {
435-
class_origins.entry(fqn).or_insert(origin);
436-
}
437-
for fqn in symbols.functions {
438-
function_origins.entry(fqn).or_insert(origin);
439-
}
440-
for name in symbols.constants {
441-
constant_origins.entry(name).or_insert(origin);
442-
}
443-
}
444-
result.class_origins = class_origins;
445-
result.function_origins = function_origins;
446-
result.constant_origins = constant_origins;
447-
result
406+
// Phase 2: scan all collected files in parallel. Each file's origin is
407+
// already known from the package it was collected under (phase 1), so
408+
// it travels alongside the path and gets attached to the functions and
409+
// constants discovered in the same read, instead of re-reading every
410+
// file in a second sequential pass just to classify it.
411+
let mut all_files: Vec<(PathBuf, crate::ClassCompletionOrigin)> = psr4_files
412+
.into_iter()
413+
.map(|(path, _, origin)| (path, origin))
414+
.collect();
415+
all_files.extend(plain_files);
416+
417+
progress_add_total(progress, all_files.len());
418+
419+
scan_files_parallel_full(&all_files, progress)
448420
}
449421

450422
/// Scan all `.php` files under the workspace root using the PSR-4
@@ -609,8 +581,14 @@ fn scan_files_parallel_psr4(
609581

610582
/// Scan a batch of files for all symbols (classes, functions, constants)
611583
/// in parallel and return a [`WorkspaceScanResult`].
584+
///
585+
/// Each file carries its own completion-origin tier, which is attached to
586+
/// any function or constant discovered in it. This lets callers that know
587+
/// a file's package provenance up front (e.g. vendor package scanning)
588+
/// classify symbols in the same read/scan pass instead of re-reading every
589+
/// file afterwards just to determine its origin.
612590
fn scan_files_parallel_full(
613-
files: &[PathBuf],
591+
files: &[(PathBuf, crate::ClassCompletionOrigin)],
614592
progress: Option<&ScanProgress>,
615593
) -> WorkspaceScanResult {
616594
if files.is_empty() {
@@ -620,7 +598,7 @@ fn scan_files_parallel_full(
620598
// Small batches: sequential
621599
if files.len() <= 4 {
622600
let mut result = WorkspaceScanResult::default();
623-
for path in files {
601+
for (path, origin) in files {
624602
progress_add_done(progress);
625603
if let Ok(content) = read_for_scan(path) {
626604
let scan = super::find_symbols(&content);
@@ -642,14 +620,16 @@ fn scan_files_parallel_full(
642620
for fqn in scan.functions {
643621
result
644622
.function_index
645-
.entry(fqn)
623+
.entry(fqn.clone())
646624
.or_insert_with(|| path.clone());
625+
result.function_origins.entry(fqn).or_insert(*origin);
647626
}
648627
for name in scan.constants {
649628
result
650629
.constant_index
651-
.entry(name)
630+
.entry(name.clone())
652631
.or_insert_with(|| path.clone());
632+
result.constant_origins.entry(name).or_insert(*origin);
653633
}
654634
}
655635
}
@@ -659,42 +639,44 @@ fn scan_files_parallel_full(
659639
let n_threads = thread_count().min(files.len());
660640
let chunk_size = files.len().div_ceil(n_threads);
661641

662-
let results: Vec<Vec<(ScanResult, PathBuf)>> = std::thread::scope(|s| {
663-
let handles: Vec<_> = files
664-
.chunks(chunk_size)
665-
.map(|chunk| {
666-
s.spawn(move || {
667-
let mut local: Vec<(ScanResult, PathBuf)> = Vec::new();
668-
for path in chunk {
669-
progress_add_done(progress);
670-
if let Ok(content) = read_for_scan(path) {
671-
let scan = super::find_symbols(&content);
672-
if !scan.classes.is_empty()
673-
|| !scan.functions.is_empty()
674-
|| !scan.constants.is_empty()
675-
{
676-
local.push((scan, path.clone()));
642+
let results: Vec<Vec<(ScanResult, PathBuf, crate::ClassCompletionOrigin)>> =
643+
std::thread::scope(|s| {
644+
let handles: Vec<_> = files
645+
.chunks(chunk_size)
646+
.map(|chunk| {
647+
s.spawn(move || {
648+
let mut local: Vec<(ScanResult, PathBuf, crate::ClassCompletionOrigin)> =
649+
Vec::new();
650+
for (path, origin) in chunk {
651+
progress_add_done(progress);
652+
if let Ok(content) = read_for_scan(path) {
653+
let scan = super::find_symbols(&content);
654+
if !scan.classes.is_empty()
655+
|| !scan.functions.is_empty()
656+
|| !scan.constants.is_empty()
657+
{
658+
local.push((scan, path.clone(), *origin));
659+
}
677660
}
678661
}
679-
}
680-
local
662+
local
663+
})
681664
})
682-
})
683-
.collect();
684-
handles
685-
.into_iter()
686-
.map(|h| {
687-
h.join().unwrap_or_else(|_| {
688-
tracing::error!("PHPantom: thread panic in scan_files_parallel_full");
689-
Vec::new()
665+
.collect();
666+
handles
667+
.into_iter()
668+
.map(|h| {
669+
h.join().unwrap_or_else(|_| {
670+
tracing::error!("PHPantom: thread panic in scan_files_parallel_full");
671+
Vec::new()
672+
})
690673
})
691-
})
692-
.collect()
693-
});
674+
.collect()
675+
});
694676

695677
let mut result = WorkspaceScanResult::default();
696678
for batch in results {
697-
for (scan, path) in batch {
679+
for (scan, path, origin) in batch {
698680
for fqcn in scan.classes {
699681
let class_short_name = fqcn_short_name(&fqcn).to_owned();
700682
result
@@ -719,14 +701,16 @@ fn scan_files_parallel_full(
719701
for fqn in scan.functions {
720702
result
721703
.function_index
722-
.entry(fqn)
704+
.entry(fqn.clone())
723705
.or_insert_with(|| path.clone());
706+
result.function_origins.entry(fqn).or_insert(origin);
724707
}
725708
for name in scan.constants {
726709
result
727710
.constant_index
728-
.entry(name)
711+
.entry(name.clone())
729712
.or_insert_with(|| path.clone());
713+
result.constant_origins.entry(name).or_insert(origin);
730714
}
731715
}
732716
}
@@ -778,11 +762,11 @@ pub fn scan_workspace_fallback_full(
778762
})
779763
.build();
780764

781-
let mut php_files: Vec<PathBuf> = Vec::new();
765+
let mut php_files: Vec<(PathBuf, crate::ClassCompletionOrigin)> = Vec::new();
782766
for entry in walker.flatten() {
783767
let path = entry.path();
784768
if path.is_file() && path.extension().is_some_and(|ext| ext == "php") {
785-
php_files.push(path.to_path_buf());
769+
php_files.push((path.to_path_buf(), crate::ClassCompletionOrigin::Project));
786770
}
787771
}
788772

@@ -822,7 +806,7 @@ pub fn scan_drupal_directories(
822806
"sites",
823807
];
824808

825-
let mut php_files: Vec<PathBuf> = Vec::new();
809+
let mut php_files: Vec<(PathBuf, crate::ClassCompletionOrigin)> = Vec::new();
826810

827811
for rel in &drupal_dirs {
828812
let dir = web_root.join(rel);
@@ -855,7 +839,7 @@ pub fn scan_drupal_directories(
855839
for entry in walker.flatten() {
856840
let path = entry.path();
857841
if path.is_file() && is_drupal_php_file(path) {
858-
php_files.push(path.to_path_buf());
842+
php_files.push((path.to_path_buf(), crate::ClassCompletionOrigin::Project));
859843
}
860844
}
861845
}

src/classmap_scanner/mod.rs

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -152,8 +152,6 @@ pub struct ScanResult {
152152
pub struct WorkspaceScanResult {
153153
/// FQN → file path for classes, interfaces, traits, and enums.
154154
pub classmap: HashMap<String, PathBuf>,
155-
/// FQN → completion origin tier.
156-
pub(crate) class_origins: HashMap<String, crate::ClassCompletionOrigin>,
157155
/// FQN → file path for standalone functions.
158156
pub function_index: HashMap<String, PathBuf>,
159157
/// FQN → completion origin tier for standalone functions.

0 commit comments

Comments
 (0)