[ty] Short circuit ReachabilityConstraints::analyze_single for dynamic types
#19867
Conversation
MichaReiser
requested review from
AlexWaygood,
sharkdp and
dcreager
as code owners
MichaReiser
added
performance
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
CodSpeed WallTime Performance ReportMerging #19867 will not alter performanceComparing Summary
|
|
Nice, this is even a win for non concurrent runs. All credit goes to @carljm for the much better fix than what I came up with |
| // doesn't help much. | ||
| // See <https://github.com/astral-sh/ty/issues/968>. | ||
| if matches!(ty, Type::Dynamic(_)) { | ||
| return Truthiness::AlwaysTrue.negate_if(!predicate.is_positive); |
There was a problem hiding this comment.
Sorry, I gave you the wrong info here, I had the sense of this constraint reversed in my head. This needs to return Truthiness::AlwaysFalse, not Truthiness::AlwaysTrue! I suspect the mypy-primer run is taking so long because there are now tons more bogus diagnostics 😆
The fact that no test failed on this change also suggests we need an explicit mdtest like this:
from typing import Never, Any
def f(func: Any) -> Never:
func()Where an invalid-return-type is expected (because the call to func() is not terminal). That test fails on this PR currently.
| return Truthiness::AlwaysTrue.negate_if(!predicate.is_positive); | |
| return Truthiness::AlwaysFalse.negate_if(!predicate.is_positive); |
There was a problem hiding this comment.
Don't worry, it does. The most congested lock is now for IntersectionType:
And the wall clock time on the large project on my linux machine goes from 42s .. to 16s which is rather ridiculous
|
|
Okay, the single threaded benchmarks don't improve by that much anymore. But it's still a huge improvement for multi threaded workloads (with many cores... not what github actions provides) |
* main: Don't cache files with diagnostics (#19869) [ty] support recursive type aliases (#19805) [ty] Remove unsafe `salsa::Update` implementations in `tuple.rs` (#19880) [ty] Function argument inlay hints (#19269) [ty] Remove Salsa interning for `TypedDictType` (#19879) Fix `lint.future-annotations` link (#19876) [ty] Reduce memory usage of `TupleSpec` and `TupleType` (#19872) [ty] Track heap usage of salsa structs (#19790) Update salsa to pull in tracked struct changes (#19843) [ty] simplify CycleDetector::visit signature (#19873) [ty] use interior mutability in type visitors (#19871) [ty] Fix tool name is None when no ty path is given in ty_benchmark (#19870) [ty] Remove `Type::Tuple` (#19669) [ty] Short circuit `ReachabilityConstraints::analyze_single` for dynamic types (#19867)
Summary
Fixes astral-sh/ty#968
This reduces the wall time for a large project from ~24s to ~19s. I plan to do another perf recording to verify that the lock congestion is entirely gone but I first need to walk the dog :)
See inline comment for more details.
Test Plan
cargo nextest