Skip to content

Conversation

@sharkdp
Copy link
Contributor

@sharkdp sharkdp commented Feb 20, 2025

Summary

#16121 + #16268

Test Plan

@sharkdp sharkdp added the ty Multi-file analysis & type inference label Feb 20, 2025
@codspeed-hq
Copy link

codspeed-hq bot commented Feb 20, 2025

CodSpeed Performance Report

Merging #16279 will degrade performances by 24.12%

Comparing david/descriptor-protocol-merge-16268 (9b93739) with main (b385c7d)

Summary

❌ 2 regressions
✅ 30 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark BASE HEAD Change
red_knot_check_file[cold] 91.7 ms 101.3 ms -9.51%
red_knot_check_file[incremental] 5.7 ms 7.6 ms -24.12%

@sharkdp
Copy link
Contributor Author

sharkdp commented Feb 20, 2025

The first attempt is worse than my PR alone:

Benchmark BASE HEAD Change
red_knot_check_file[cold] 91.7 ms 104.6 ms -12.39%
red_knot_check_file[incremental] 5.7 ms 8.2 ms -29.93%

@sharkdp
Copy link
Contributor Author

sharkdp commented Feb 20, 2025

It looks better when symbol_by_id is a query again:

Benchmark BASE HEAD Change
red_knot_check_file[cold] 91.7 ms 101.3 ms -9.5%
red_knot_check_file[incremental] 5.7 ms 7.6 ms -24.12%

@MichaReiser
Copy link
Member

That's good to know. It shows that symbol_by_id being a query is important

@MichaReiser
Copy link
Member

It now makes me wonder if #16265 would perform even better?

@sharkdp
Copy link
Contributor Author

sharkdp commented Feb 20, 2025

It now makes me wonder if #16265 would perform even better?

No: #16280 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ty Multi-file analysis & type inference

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants