Add fast path to TypeCombinator::union() for trivial 2-type cases#5325
Conversation
|
I tried similar things in the past. in the end I did not propose the patch, because - while it seems useful in microbenchmarks - I was not able to prove on real world code-bases that it makes a meaningful difference. did you run this change on your projects and did it make a difference? |
I haven't run it yet on my own projects. This is the result from the benchmarks in the CI of this PR (https://github.com/phpstan/phpstan-src/actions/runs/23704938111/job/69055142257?pr=5325 and
No benchmark regressed beyond noise. Consistent small improvements across the board, with bug-10772 showing a notable -25%. I will see if I can run this change on 2 of the bigger projects I work on |
I saw a 2.6% and 2.3% speed up on my 2 bigger projects. So I think it will mainly benefit specific setups like we see in the benchmarks, some bugs receiving 5-25% performance improvement while others show noise level changes. |
|
Thank you! This reminded me of something I noticed when I was profiling TypeCombinator some time a go. When I do this: diff --git a/src/Type/TypeCombinator.php b/src/Type/TypeCombinator.php
index 46dad7268a..b7ee1190c2 100644
--- a/src/Type/TypeCombinator.php
+++ b/src/Type/TypeCombinator.php
@@ -180,6 +180,12 @@ final class TypeCombinator
}
}
+ foreach ($types as $type) {
+ var_dump($type->describe(VerbosityLevel::precise()));
+ }
+
+ var_dump('------');
+
$alreadyNormalized = [];
$alreadyNormalizedCounter = 0;
The output is full of cases like this: So a lot of unions with many NeverType instances. I'll try to optimize your branch further with this knowledge. |
ondrejmirtes
force-pushed
the
perf/typecombinator-union-fast-path
branch
from
f2b7b6c to
fe1df1a
Compare
|
Thank you! |
Seems like a nice improvement, it made one bug benchmark case over 90% faster! However, it also made a lot of benchmarks slightly slower. I've made an iteration on it that should reduce this overhead. However, I don't understand the implications not remotely as well as you do, so I'm looking forward to your review of #5374
|
3 participants
Problem
TypeCombinator::union()is one of the most called methods in PHPStan. Profiling shows 30k-66k calls per file on heavy benchmarks. Every call goes through the full normalization pipeline: flattening nested unions, classifying scalars/arrays/enums, sorting integer ranges, O(n^2) deduplication, and array processing.Many of these calls are trivial 2-type cases where the result is immediately known:
union(NeverType, X)— result isX(never is the identity element)union(X, MixedType)— result isMixedType(mixed absorbs everything)union(X, X)— result isX(same object passed twice)These patterns are common in type narrowing, scope merging, and conditional type resolution.
Solution
Add a fast path before the main normalization loop that handles exactly these three cases when
$typesCount === 2. Each check is a cheapinstanceofor identity comparison.The checks mirror logic that already exists deeper in the method (never removal at line 199, mixed short-circuit at line 191, dedup at line 350) but avoids all the setup work.
Impact
Benchmarked on the 5 slowest files from
tests/bench/data/, measured in isolation (without other optimizations):9,363ms → 7,893ms (-15.7%)
All files benefit roughly equally since
union()is called pervasively.