Improve performance of custom-iterator __getitem__
#1096
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Flipping the order of `Slice` and `Int` in `SliceOrInt` so that `Int` comes first means that the `FromPyObject` derivation will then try `Int` first, which is the correct variant in like 99.9% of uses of the struct. This has the impact of improving int `__getitem__` times in the custom iterators by about 3x (from 205ns to 61ns on my machine), which has knock-on effects for the implicit iterators Python defines for these classes.
jakelishman
force-pushed
the
getitem-perf
branch
from
65a4b71
to
7a49961
Compare
Pull Request Test Coverage Report for Build 7978469504Details
💛 - Coveralls |
mtreinish
added
the
stable-backport-potential
mtreinish
approved these changes
Feb 20, 2024
mergify bot
pushed a commit
that referenced
this pull request
Feb 20, 2024
Flipping the order of `Slice` and `Int` in `SliceOrInt` so that `Int` comes first means that the `FromPyObject` derivation will then try `Int` first, which is the correct variant in like 99.9% of uses of the struct. This has the impact of improving int `__getitem__` times in the custom iterators by about 3x (from 205ns to 61ns on my machine), which has knock-on effects for the implicit iterators Python defines for these classes. (cherry picked from commit d4f28e3)
|
@Mergifyio backport stable/0.14 |
✅ Backports have been created
|
mergify bot
pushed a commit
that referenced
this pull request
Feb 20, 2024
Flipping the order of `Slice` and `Int` in `SliceOrInt` so that `Int` comes first means that the `FromPyObject` derivation will then try `Int` first, which is the correct variant in like 99.9% of uses of the struct. This has the impact of improving int `__getitem__` times in the custom iterators by about 3x (from 205ns to 61ns on my machine), which has knock-on effects for the implicit iterators Python defines for these classes. (cherry picked from commit d4f28e3)
|
Thanks for writing this! I had a feeling we’d need to profile the function but I guess the fix was easier than we thought |
mtreinish
pushed a commit
that referenced
this pull request
Feb 21, 2024
Flipping the order of `Slice` and `Int` in `SliceOrInt` so that `Int` comes first means that the `FromPyObject` derivation will then try `Int` first, which is the correct variant in like 99.9% of uses of the struct. This has the impact of improving int `__getitem__` times in the custom iterators by about 3x (from 205ns to 61ns on my machine), which has knock-on effects for the implicit iterators Python defines for these classes. (cherry picked from commit d4f28e3) Co-authored-by: Jake Lishman <jake.lishman@ibm.com>
mtreinish
added a commit
to mtreinish/retworkx
that referenced
this pull request
Feb 22, 2024
This commit prepares for a 0.14.1 release which just includes Qiskit#1109 to fix ppc64le builds and Qiskit#1096 to fix the overhead of `__getitem__` on custom sequence return types.
Merged
mtreinish
added a commit
that referenced
this pull request
Feb 22, 2024
* Prepare 0.14.1 release This commit prepares for a 0.14.1 release which just includes #1109 to fix ppc64le builds and #1096 to fix the overhead of `__getitem__` on custom sequence return types. * Update releasenotes/notes/prepare-0.14.1-e5065553a44eb035.yaml Co-authored-by: Ivan Carvalho <8753214+IvanIsCoding@users.noreply.github.com> * Update version in docs config too --------- Co-authored-by: Ivan Carvalho <8753214+IvanIsCoding@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Flipping the order of
SliceandIntinSliceOrIntso thatIntcomes first means that theFromPyObjectderivation will then tryIntfirst, which is the correct variant in like 99.9% of uses of the struct. This has the impact of improving int__getitem__times in the custom iterators by about 3x (from 205ns to 61ns on my machine), which has knock-on effects for the implicit iterators Python defines for these classes.This implements the performance improvement I mentioned in #1090 (comment). I can also look into defining manual Python-space iterators (rather than using the implicit one based on
__getitem__), if that's an interesting path - I haven't tried it yet, so I don't know if there's more performance to be had.