Skip to content

[mypyc] Speed up and improve multiple assignment #9800

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

Merged
merged 9 commits into from
Dec 29, 2020
Merged

Conversation

JukkaL
Copy link
Collaborator

@JukkaL JukkaL commented Dec 13, 2020

Speed up multiple assignment from variable-length lists and tuples.
This speeds up the multiple_assignment benchmark by around 80%.

Fix multiple lvalues in fixed-length sequence assignments.

Optimize some cases of list expressions in assignments.

Fixes mypyc/mypyc#729.

@JukkaL JukkaL requested a review from TH3CHARLie December 13, 2020 18:40
@msullivan msullivan self-requested a review December 13, 2020 20:09
Copy link
Collaborator

@msullivan msullivan left a comment

Choose a reason for hiding this comment

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

Looks good, nice improvement.

I made a comment about some evaluation order stuff, but it should be fine to merge this before coming to a final decision about that sort of thing.


# Assign sequence items to the target lvalues.
for lvalue, value in zip(target.items, values):
self.assign(lvalue, value, line)
Copy link
Collaborator

Choose a reason for hiding this comment

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

The iterator code below assigns as it goes. Do we know which Python does? It's all pretty marginal, and if we wanted to say we aren't aiming to exactly replicate execution order in some marginal cases, that's probably fine, but we should do it with eyes open. (In general I've tried to match things but I know we mismatch in at least one case with method calls.)

I guess in this case we wouldn't be able to do unsafe get items if we were assigning as we went (because it could get modified.)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

CPython reads all the values before performing the assignments, so this brings the semantics a bit closer to Python. I'll create an issue about updating the generic iterable case to also match Python semantics.

Copy link
Collaborator

@TH3CHARLie TH3CHARLie left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks for fixing the issue!

@JukkaL JukkaL merged commit 18ab589 into master Dec 29, 2020
@JukkaL JukkaL deleted the multi-assign branch December 29, 2020 11:56
JukkaL added a commit that referenced this pull request Dec 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make multiple assignment faster
3 participants