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

Reduce size of Stmt from 144 to 120 bytes #11051

Merged
merged 1 commit into from
Apr 19, 2024
Merged

Reduce size of Stmt from 144 to 120 bytes #11051

merged 1 commit into from
Apr 19, 2024

Conversation

carljm
Copy link
Contributor

@carljm carljm commented Apr 19, 2024

Summary

I happened to notice that we box TypeParams on StmtClassDef but not on StmtFunctionDef and wondered why, since StmtFunctionDef is bigger and sets the size of Stmt.

@charliermarsh found that at the time we started boxing type params on classes, classes were the largest statement type (see #6275), but that's no longer true.

So boxing type-params also on functions reduces the overall size of Stmt.

Test Plan

The <= size tests are a bit irritating (since their failure doesn't tell you the actual size), but I manually confirmed that the size is actually 120 now.

@carljm carljm added the internal An internal refactor or improvement label Apr 19, 2024
@carljm carljm requested a review from charliermarsh April 19, 2024 22:45
Copy link

codspeed-hq bot commented Apr 19, 2024

CodSpeed Performance Report

Merging #11051 will improve performances by 6.13%

Comparing box-type-params (74b2d51) with main (99f7f94)

Summary

⚡ 1 improvements
✅ 29 untouched benchmarks

Benchmarks breakdown

Benchmark main box-type-params Change
parser[unicode/pypinyin.py] 1.7 ms 1.6 ms +6.13%

Copy link
Member

@charliermarsh charliermarsh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link
Contributor

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

@carljm carljm merged commit c80b9a4 into main Apr 19, 2024
17 checks passed
@carljm carljm deleted the box-type-params branch April 19, 2024 23:02
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