Skip to content

Comments

Reduce size of Stmt from 144 to 120 bytes#11051

Merged
carljm merged 1 commit intomainfrom
box-type-params
Apr 19, 2024
Merged

Reduce size of Stmt from 144 to 120 bytes#11051
carljm merged 1 commit intomainfrom
box-type-params

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
@codspeed-hq
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!

@github-actions
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
@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