Skip to content

Commit 579bce9

Browse files
committed
Type narrowing against @phpstan-assert/@psalm-assert no longer leaks
memory
1 parent c68b0c0 commit 579bce9

5 files changed

Lines changed: 159 additions & 35 deletions

File tree

docs/CHANGELOG.md

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

5353
### Fixed
5454

55+
- **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.
5556
- **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.
5657
- **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.
5758
- **Linked editing no longer types into unrelated parts of the file.** Adding a line above a variable (for example inserting a `/** */` docblock) and then typing could insert the typed characters at two arbitrary places further down, such as in the middle of a method name and in front of a trailing comment. PHPantom is re-reading the file in the background while you type, and linked editing was handing the editor positions measured against the previous version of the file, which the editor then edited on trust. Linked editing now verifies that the positions it reports still point at the variable, and offers nothing at all if they do not, so a keystroke can never land somewhere unexpected. It also becomes available in Blade templates, where the reported positions are checked against the template itself rather than the generated PHP.

docs/todo/performance.md

Lines changed: 50 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1182,14 +1182,56 @@ trimmed live figure, confirming arena count was never the lever.
11821182

11831183
What is left is allocation *count* itself: at ~45 B of overhead per
11841184
live allocation, each allocation avoided is worth more than its own
1185-
`size_of`. P39 removes hundreds of thousands of allocations as a side
1186-
effect (interning `SymbolKind` strings), so re-measure this line after
1187-
it lands. If a meaningful gap remains afterward, the next lever is
1188-
arena-allocating the member metadata (bump-allocate `MethodInfo`/`PropertyInfo`/
1189-
`ParameterInfo` per resolved class instead of one `Box`/`Arc` per
1190-
member) — a much larger change than anything else in this list, so only
1191-
worth it if the per-allocation overhead is still the dominant cost once
1192-
the count itself has come down.
1185+
`size_of`. P39 removed hundreds of thousands of allocations as a side
1186+
effect (interning `SymbolKind` strings).
1187+
1188+
**2026-07-27 re-measurement.** With P39 landed, re-running the
1189+
`mem-audit` walk turned up a genuine leak in the assertion-narrowing
1190+
path (`src/type_engine/types/narrowing/assertions.rs`): every
1191+
`@phpstan-assert`/`@psalm-assert` call-site evaluation `Box::leak`ed a
1192+
cloned `FunctionInfo` or `MethodInfo` to get a stable reference for
1193+
`CallAssertionInfo`, and neither clone was ever freed. Narrowing runs
1194+
on every conditional the forward walker visits, so this leaked once
1195+
per asserting call touched during a pass, for the life of the
1196+
process — worse in a long-running LSP session (re-run on every
1197+
keystroke) than in one-shot `analyze`. Fixed by having
1198+
`CallAssertionInfo` own the four fields it actually needs (an
1199+
`Arc`-shared parameter vector plus three small `Vec`s) instead of
1200+
leaking the whole clone.
1201+
1202+
Re-measured on four real projects (one small, two mid-size, one
1203+
large) with the fixed build: live allocations dropped 7-12% on the
1204+
three larger projects and 53% on the small one, which spends
1205+
proportionally more of its pass time in narrowing relative to its
1206+
total symbol count. Live bytes dropped correspondingly (tens of MB on
1207+
the larger projects). The allocator-overhead *percentage* barely
1208+
moved — mimalloc's rounding overhead per allocation is roughly
1209+
constant regardless of which allocations are removed, so cutting
1210+
count shrinks both the overhead and the total footprint by a similar
1211+
proportion. This confirms the mechanism above without yet closing the
1212+
gap enough to retire this item.
1213+
1214+
While re-measuring, the audit's drop-and-measure probes (`src/mem_audit.rs`)
1215+
were extended to cover every remaining `Backend` store (the phar
1216+
archive cache, autoload/stub indexes, workspace PSR-4/vendor paths,
1217+
diagnostic pull-model caches, and more). Previously 18-26% of live
1218+
heap was an unattributed gap between the field-level walk and the
1219+
allocator's own count; the probes now account for nearly all of it, so
1220+
future passes on this item measure against a real residual (dominated
1221+
by the `ustr` name interner and short-lived per-request state) instead
1222+
of an unexplained shortfall. One find from the newly-covered stores:
1223+
`phar_archives` (raw phar bytes plus a byte-offset index, kept for lazy
1224+
re-extraction of phar-embedded PHP files such as vendor-shipped
1225+
PHPStan) held on the order of 25 MB on projects with a phar-shaped
1226+
dependency, previously invisible to the audit.
1227+
1228+
If a meaningful gap remains once other in-flight allocation-count
1229+
reductions land, the next lever is arena-allocating the member
1230+
metadata (bump-allocate `MethodInfo`/`PropertyInfo`/`ParameterInfo` per
1231+
resolved class instead of one `Box`/`Arc` per member) — a much larger
1232+
change than anything else in this list, and today's re-measurement
1233+
does not yet show the overhead percentage moving enough to justify it
1234+
on its own.
11931235

11941236
---
11951237

src/mem_audit.rs

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1579,6 +1579,88 @@ pub(crate) fn report(backend: &Backend, runner_content_bytes: usize) {
15791579
probe("global_functions", &mut || {
15801580
backend.symbols.global_functions.write().clear()
15811581
});
1582+
probe("global_defines", &mut || {
1583+
backend.symbols.global_defines.write().clear()
1584+
});
1585+
probe("gti_index", &mut || {
1586+
backend.symbols.gti_index.write().clear()
1587+
});
1588+
probe("uri_globals_index", &mut || {
1589+
backend.symbols.uri_globals_index.write().clear()
1590+
});
1591+
probe("fqn_uri + fqn_origin index", &mut || {
1592+
backend.symbols.fqn_uri_index.write().clear();
1593+
backend.symbols.fqn_origin_index.write().clear();
1594+
});
1595+
probe("autoload indexes", &mut || {
1596+
backend.symbols.autoload_function_index.write().clear();
1597+
backend
1598+
.symbols
1599+
.autoload_function_origin_index
1600+
.write()
1601+
.clear();
1602+
backend.symbols.autoload_constant_index.write().clear();
1603+
backend
1604+
.symbols
1605+
.autoload_constant_origin_index
1606+
.write()
1607+
.clear();
1608+
backend.symbols.autoload_file_paths.write().clear();
1609+
});
1610+
probe("class_not_found_cache", &mut || {
1611+
backend.symbols.class_not_found_cache.write().clear()
1612+
});
1613+
probe("stub indexes", &mut || {
1614+
backend.stub_index.write().clear();
1615+
backend.stub_function_index.write().clear();
1616+
backend.stub_constant_index.write().clear();
1617+
});
1618+
probe("parsed_uris", &mut || backend.parsed_uris.write().clear());
1619+
probe("laravel_aliases", &mut || {
1620+
*backend.laravel_aliases.write() = None
1621+
});
1622+
probe("laravel seed/mixin/pivot/command sets", &mut || {
1623+
backend.laravel_macro_seeds.write().clear();
1624+
backend.laravel_macro_mixin_uris.write().clear();
1625+
backend.laravel_date_seed_uris.write().clear();
1626+
*backend.laravel_pivots.write() = Default::default();
1627+
*backend.laravel_commands.write() = Default::default();
1628+
});
1629+
probe("blade_uris", &mut || backend.blade_uris.write().clear());
1630+
probe("phar_archives", &mut || {
1631+
backend.phar_archives.write().clear()
1632+
});
1633+
probe("open_files", &mut || backend.open_files.write().clear());
1634+
probe("did_change_parse_locks", &mut || {
1635+
backend.did_change_parse_locks.lock().clear()
1636+
});
1637+
probe("workspace psr4 + vendor paths", &mut || {
1638+
backend.workspace.psr4_mappings.write().clear();
1639+
backend.workspace.vendor_uri_prefixes.lock().clear();
1640+
backend.workspace.vendor_dir_paths.lock().clear();
1641+
backend
1642+
.workspace
1643+
.vendor_package_origin_roots
1644+
.write()
1645+
.clear();
1646+
});
1647+
probe("diagnostic state", &mut || {
1648+
backend.diag.pending_uris.lock().clear();
1649+
backend.diag.last_slow.lock().clear();
1650+
backend.diag.last_fast.lock().clear();
1651+
backend.diag.last_full.lock().clear();
1652+
backend.diag.result_ids.lock().clear();
1653+
backend.diag.suppressed.lock().clear();
1654+
*backend.diag.workspace_diags.lock() = Default::default();
1655+
});
1656+
1657+
let (residual_bytes, residual_allocs) = live_heap();
1658+
eprintln!(
1659+
" ── residual after every probe: {:.1} MB in {} allocations (runner file contents {:.1} MB; the rest is ustr, thread-local caches, and runtime state)",
1660+
mb(residual_bytes),
1661+
residual_allocs,
1662+
mb(runner_content_bytes),
1663+
);
15821664

15831665
eprintln!(
15841666
"── audited live total {:.1} MB (cache section {:.1} MB) | VmRSS after collect {:.1} MB | VmHWM {:.1} MB",

src/type_engine/types/narrowing/assertions.rs

Lines changed: 24 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use std::sync::Arc;
66

77
use crate::atom::{Atom, atom, bytes_to_str};
88
use crate::php_type::PhpType;
9-
use crate::types::{AssertionKind, ClassInfo, ParameterInfo, TypeAssertion};
9+
use crate::types::{AssertionKind, ClassInfo, ParameterInfo, SharedVec, TypeAssertion};
1010

1111
use mago_span::HasSpan;
1212
use mago_syntax::cst::*;
@@ -22,18 +22,24 @@ use super::*;
2222
/// Produced by [`extract_call_assertions`] so that callers can apply
2323
/// narrowing logic uniformly regardless of whether the call is
2424
/// `myFunc($x)` or `Assert::check($x)`.
25+
///
26+
/// The callee metadata is owned rather than borrowed: the callee is a
27+
/// clone produced by the loader (or by the trait/parent chain walk), so
28+
/// there is nothing outliving the call to borrow from. Moving the four
29+
/// fields out of that clone keeps the cost to an `Arc` bump plus three
30+
/// `Vec` moves, and lets the rest of the clone drop immediately.
2531
pub(in crate::type_engine) struct CallAssertionInfo<'a> {
2632
/// The `@phpstan-assert` / `@psalm-assert` annotations on the callee.
27-
pub(in crate::type_engine) assertions: &'a [TypeAssertion],
33+
pub(in crate::type_engine) assertions: Vec<TypeAssertion>,
2834
/// The callee's parameter list (used to map assertion `$param` names
2935
/// to positional argument indices).
30-
pub(in crate::type_engine) parameters: &'a [ParameterInfo],
36+
pub(in crate::type_engine) parameters: SharedVec<ParameterInfo>,
3137
/// The call-site argument list.
3238
pub(in crate::type_engine) argument_list: &'a ArgumentList<'a>,
3339
/// Template parameter names from the callee's `@template` tags.
34-
template_params: &'a [Atom],
40+
template_params: Vec<Atom>,
3541
/// Template parameter → parameter name bindings (e.g. `("T", "$class")`).
36-
template_bindings: &'a [(Atom, Atom)],
42+
template_bindings: Vec<(Atom, Atom)>,
3743
}
3844

3945
/// Try to extract assertion metadata from a call expression.
@@ -61,16 +67,12 @@ pub(in crate::type_engine) fn extract_call_assertions<'a>(
6167
if func_info.type_assertions.is_empty() {
6268
return None;
6369
}
64-
// SAFETY: We leak the FunctionInfo to get a stable reference.
65-
// This is acceptable because narrowing runs once per completion
66-
// request and the allocation is small.
67-
let func_info = Box::leak(Box::new(func_info));
6870
Some(CallAssertionInfo {
69-
assertions: &func_info.type_assertions,
70-
parameters: &func_info.parameters,
71+
assertions: func_info.type_assertions,
72+
parameters: func_info.parameters,
7173
argument_list: &func_call.argument_list,
72-
template_params: &func_info.template_params,
73-
template_bindings: &func_info.template_bindings,
74+
template_params: func_info.template_params,
75+
template_bindings: func_info.template_bindings,
7476
})
7577
}
7678
Call::StaticMethod(static_call) => {
@@ -170,15 +172,12 @@ fn build_method_assertion_info<'a>(
170172
) -> Option<CallAssertionInfo<'a>> {
171173
let method =
172174
find_assertion_method_in_chain(class, method_name, ctx.class_loader, &mut Vec::new(), 0)?;
173-
// Leak MethodInfo to get a stable reference for the duration of this
174-
// narrowing pass.
175-
let method = Box::leak(Box::new(method));
176175
Some(CallAssertionInfo {
177-
assertions: &method.type_assertions,
178-
parameters: &method.parameters,
176+
assertions: method.type_assertions,
177+
parameters: method.parameters,
179178
argument_list,
180-
template_params: &method.template_params,
181-
template_bindings: &method.template_bindings,
179+
template_params: method.template_params,
180+
template_bindings: method.template_bindings,
182181
})
183182
}
184183

@@ -324,12 +323,12 @@ pub(in crate::type_engine) fn try_apply_custom_assert_narrowing(
324323
None => return false,
325324
};
326325
let mut definite = false;
327-
for assertion in info.assertions {
326+
for assertion in &info.assertions {
328327
if assertion.kind != AssertionKind::Always {
329328
continue;
330329
}
331330
if let Some(arg_var) =
332-
find_assertion_arg_variable(info.argument_list, &assertion.param_name, info.parameters)
331+
find_assertion_arg_variable(info.argument_list, &assertion.param_name, &info.parameters)
333332
&& arg_var == ctx.var_name
334333
{
335334
// Resolve the asserted type. When the type is a template
@@ -404,7 +403,7 @@ pub(in crate::type_engine) fn collect_assert_reexport_conditions<'a>(
404403
return Vec::new();
405404
};
406405
let mut out = Vec::new();
407-
for assertion in info.assertions {
406+
for assertion in &info.assertions {
408407
if assertion.kind != AssertionKind::Always {
409408
continue;
410409
}
@@ -420,7 +419,7 @@ pub(in crate::type_engine) fn collect_assert_reexport_conditions<'a>(
420419
continue;
421420
};
422421
if let Some(arg_expr) =
423-
assertion_arg_expression(info.argument_list, &assertion.param_name, info.parameters)
422+
assertion_arg_expression(info.argument_list, &assertion.param_name, &info.parameters)
424423
{
425424
out.push((arg_expr, asserts_true));
426425
}
@@ -476,7 +475,7 @@ pub(in crate::type_engine) fn call_asserts_not_null(
476475
&& find_assertion_arg_variable(
477476
info.argument_list,
478477
&assertion.param_name,
479-
info.parameters,
478+
&info.parameters,
480479
)
481480
.as_deref()
482481
== Some(ctx.var_name)

src/type_engine/types/narrowing/guards.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -373,7 +373,7 @@ pub(in crate::type_engine) fn apply_guard_clause_narrowing(
373373
// inverted=true, same logic as try_apply_assert_condition_narrowing
374374
let function_returned_true = condition_negated;
375375

376-
for assertion in info.assertions {
376+
for assertion in &info.assertions {
377377
let applies_positively = match assertion.kind {
378378
AssertionKind::IfTrue => function_returned_true,
379379
AssertionKind::IfFalse => !function_returned_true,
@@ -383,7 +383,7 @@ pub(in crate::type_engine) fn apply_guard_clause_narrowing(
383383
if let Some(arg_var) = find_assertion_arg_variable(
384384
info.argument_list,
385385
&assertion.param_name,
386-
info.parameters,
386+
&info.parameters,
387387
) && arg_var == ctx.var_name
388388
{
389389
let should_exclude = assertion.negated ^ !applies_positively;

0 commit comments

Comments
 (0)