-
-
Notifications
You must be signed in to change notification settings - Fork 3k
[mypyc] feat: cache len for iterating over immutable types #19656
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
for more information, see https://pre-commit.ci
…/mypy into for-loop-immutable
for more information, see https://pre-commit.ci
L0: | ||
r0 = 'abc' | ||
source = r0 | ||
r1 = CPyStr_Size_size_t(source) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this CPyStr_Size twice in 4 lines thing looks pretty ugly, but its better than what we had before. This will be optimized a bit better in my follow-up PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks reasonable. I suspect that this will rarely yield any significant performance improvement, but the implementation is simple enough and it could help in the future more once we optimize other things (almost all the change volume is in irbuild tests, which are easier to maintain since they can be updated automatically).
I did see a ~0.4% improvement when I benchmarked self-check, but that was on the larger PR with the other len-related change as well. In any case the IR looks prettier now! |
Currently, if a user uses an immutable type as the sequence input for a for loop, the length is checked once at each iteration which, while necessary for some container types such as list and dictionaries, is not necessary for iterating over immutable types tuple, str, and bytes.
This PR modifies the codebase such that the length is only checked at the first iteration, and reused from there.