-
-
Notifications
You must be signed in to change notification settings - Fork 3k
[mypyc] Faster set creation with generator expression #10261
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
[mypyc] Faster set creation with generator expression #10261
Conversation
Thanks for the PR! Have you benchmarked this against master? (If not, no worries, I can do some quick benchmarking tomorrow.) |
Not yet. |
@@ -143,6 +143,9 @@ def translate_safe_generator_call( | |||
val = tuple_from_generator_helper(builder, expr.args[0]) | |||
if val is not None: | |||
return val | |||
if callee.fullname == "builtins.set": | |||
return translate_set_comprehension(builder, expr.args[0]) | |||
|
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.
Maybe I should move builtins.set
to a new specialize function ?
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.
That sounds reasonable, since the implementation isn't sharing much with the other cases.
I measured the performance and now the speed of calling |
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.
Thanks, looks good! This is a nice performance win.
[mypyc] Faster set creation with generator expression (python#10261)
Description
Closes mypyc/mypyc#771 and #9707
Changes:
tranform_set_comprehension
totranslate_set_comprehension
set(f(x) for x in it)
no longer uses list comprehension (#771) #9707 (comment))Test Plan
Both generated IR test and functional test:
list
tuple
dict
range
)