Skip to content
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

Optimize some SemanticModel methods #13091

Merged
merged 1 commit into from
Sep 2, 2024
Merged

Conversation

AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Aug 25, 2024

No description provided.

Copy link
Contributor

github-actions bot commented Aug 25, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

@AlexWaygood AlexWaygood force-pushed the optimize-current-exprs branch 2 times, most recently from 87567a4 to fc43cd0 Compare August 25, 2024 18:13
@AlexWaygood
Copy link
Member Author

AlexWaygood commented Aug 25, 2024

The premise of this PR is that in Python, a statement can contain a statement, an expression can contain an expression, and a statement can contain an expression -- but an expression cannot contain a statement. As such, for various methods that iterate through all "parent expression nodes" in the AST, we can stop as soon as we hit a statement node, since we know that there can be no parent expression nodes of that statement node.

It seems like this has no impact on the Codspeed microbenchmarks, but it still feels to me like the "right" thing to do regardless, if you consider Python's semantics... anyway, I'll let others judge whether it's worth it!

@AlexWaygood AlexWaygood marked this pull request as ready for review August 25, 2024 19:37
Copy link

codspeed-hq bot commented Aug 26, 2024

CodSpeed Performance Report

Merging #13091 will improve performances by 5.32%

Comparing optimize-current-exprs (2da0cb6) with main (f50f873)

Summary

⚡ 1 improvements
✅ 31 untouched benchmarks

Benchmarks breakdown

Benchmark main optimize-current-exprs Change
linter/all-rules[numpy/globals.py] 767.8 µs 729 µs +5.32%

@MichaReiser MichaReiser added the internal An internal refactor or improvement label Sep 2, 2024
@AlexWaygood AlexWaygood merged commit 58c641c into main Sep 2, 2024
20 checks passed
@AlexWaygood AlexWaygood deleted the optimize-current-exprs branch September 2, 2024 09:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal An internal refactor or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants