-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[ty] Cache some union builder queries #22052
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
CodSpeed Performance ReportMerging #22052 will improve performance by 21.48%Comparing Summary
Benchmarks breakdown
Footnotes
|
|
Curious to see the memory report. I suspect that this will regress memory usage quiet significantely and may require waiting for within-revision LRU support in salsa |
|
I also don't think we can do this if there's that significant a regression on pydantic. We know that it's already one of our most pathological codebases for performance, and we've received reports from users that make use of pydantic that our performance on pydantic is significantly slowing down our performance on their code |
|
|
(those ecosystem diagnostic changes just look like our standard flakes, we're a bit nondeterministic right now...) |
6402445 to
4a4d10b
Compare
|
Now that #22044 has landed, this changed from a 15% regression on Pydantic to a 30% improvement! Locally (macOS) it's more like a 50% improvement. |
|
I think the memory use changes seem pretty minor as well? |
|
I did a quick local benchmark on Pydantic to see how this interacts with #22048 |
I feel like it's a bit hard to tell when there are multiple changes bundled together here -- the fact that certain individual commits seem to reduce memory usage a fair bit (#22065 (comment)) presumably means that other commits in this PR increase memory usage enough to offset that. |
4a4d10b to
6c2d87e
Compare
|
I'm curious to see what the codspeed results look like if this is rebased on |
Seeing if this shows anything interesting in CodSpeed.
These were hot paths in astral-sh/ty#1968 — they aren't any longer after #22030 but they do still improve performance on the original reproduction.