Skip to content
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

optimized list iterator some more #618

Merged
merged 1 commit into from
Mar 18, 2021
Merged

optimized list iterator some more #618

merged 1 commit into from
Mar 18, 2021

Conversation

omry
Copy link
Owner

@omry omry commented Mar 18, 2021

Closes #613

Second optimization pass.
This is more impressive than the 7.7 -> 8.8x recorded improvement since there seems to be a regression compares to the measurements in #533.
Speed is still not great, but I this is as close as it gets given that we are probably losing a bunch of low level optimization done on native list because we need to unbox primitive items when we iterate.

Comparing test_list_iter[small_listconfig] by itself to what is in master shows about 50% speedup:
image

When it's pitched against the native list iteration it's "only" 130 times slower now, compares to 180 times slower before:
image

By the way: here is why this is important:
https://twitter.com/jackyliang42/status/1371530444529881094

@omry omry requested review from odelalleau and Jasha10 March 18, 2021 00:45
omegaconf/listconfig.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@odelalleau odelalleau left a comment

Choose a reason for hiding this comment

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

Just one meaningful concern

(on the speedup from moving the in-line import => that's interesting, I've always wondered if there was a performance risk with these... good to know!)

omegaconf/_utils.py Outdated Show resolved Hide resolved
omegaconf/listconfig.py Show resolved Hide resolved
@omry omry force-pushed the iter-opt2 branch 2 times, most recently from 8ce5eee to 5f53897 Compare March 18, 2021 18:43
Copy link
Collaborator

@odelalleau odelalleau left a comment

Choose a reason for hiding this comment

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

Some questions

omegaconf/basecontainer.py Outdated Show resolved Hide resolved
omegaconf/listconfig.py Show resolved Hide resolved
omegaconf/basecontainer.py Outdated Show resolved Hide resolved
omegaconf/basecontainer.py Outdated Show resolved Hide resolved
@omry
Copy link
Owner Author

omry commented Mar 18, 2021

@odelalleau I am merging this, let me know if you have any other feedback.

@omry omry merged commit f221ae6 into master Mar 18, 2021
@omry omry deleted the iter-opt2 branch March 18, 2021 21:56
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.

ListConfig iteration is much slower than plain list iteration
2 participants