-
Notifications
You must be signed in to change notification settings - Fork 8.3k
⚡️ Speed up function state_dict_prefix_replace
by 127%
#7743
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
base: master
Are you sure you want to change the base?
⚡️ Speed up function state_dict_prefix_replace
by 127%
#7743
Conversation
Here's an optimized version of your Python function. The primary changes are to minimize the creation of intermediate lists and to use dictionary comprehensions for more efficient data manipulation. ### Changes and Optimizations 1. **Avoid Unneeded List Creation:** - Instead of mapping and filtering the keys in a separate step (`map` and `filter`), it is done directly in the list comprehension. 2. **Dictionary Comprehension**: - By directly assigning `out` to `{}` or `state_dict`, it forgoes unnecessary intermediate steps in the conditional initialization. 3. **In-Loop Item Assignment**. - Keys to be replaced and corresponding operations are now handled directly within loops, reducing intermediate variable assignments. This rewritten function should perform better, especially with large dictionaries, due to reduced overhead from list operations and more efficient key manipulation.
comfy/utils.py
Outdated
out[x[1]] = w | ||
out = {} if filter_keys else state_dict | ||
|
||
for old_prefix, new_prefix in replace_prefix.items(): |
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 code should be skipped if filter_keys
is false.
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.
double confirming this as the for rp in replace_prefix:
loop is outside the if-else block
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.
I have made the necessary changes, it's ready for review @ltdrdata
@comfyanonymous ready for review |
📄 127% (1.27x) speedup for
state_dict_prefix_replace
incomfy/utils.py
⏱️ Runtime :
1.61 millisecond
→710 microseconds
(best of398
runs)📝 Explanation and details
Here's an optimized version of your Python function. The primary changes are to minimize the creation of intermediate lists and to use dictionary comprehensions for more efficient data manipulation.
Changes and Optimizations
Avoid Unneeded List Creation:
map
andfilter
), it is done directly in the list comprehension.Dictionary Comprehension:
out
to{}
orstate_dict
, it forgoes unnecessary intermediate steps in the conditional initialization.In-Loop Item Assignment.
This rewritten function should perform better, especially with large dictionaries, due to reduced overhead from list operations and more efficient key manipulation.
✅ Correctness verification report:
🌀 Generated Regression Tests Details
To edit these changes
git checkout codeflash/optimize-state_dict_prefix_replace-m9js6ztz
and push.