-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
PERF: accessing sliced indexes with populated indexing engines #51738
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
Conversation
fff62b1
to
8543351
Compare
nice! this has been on my todo list for ages but was always intimidating |
30d8e4e
to
7b50a89
Compare
7b50a89
to
989b081
Compare
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.
LGTM
return type(self)._simple_new(res, name=self._name) | ||
result = type(self)._simple_new(res, name=self._name) | ||
if "_engine" in self._cache: | ||
reverse = slobj.step is not None and slobj.step < 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.
Just confirming this is still valid if slobj
is empty? slice(0,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.
Yeah, this is valid if we have e.g. slice(None)
, because then slobj.step
is always None. For slice(0,0)
the .step
attribute is None, so no problem there.
Thanks @topper-123 |
Improves performance of indexes that are sliced from indexes with already-built indexing engines by copying the relevant data from the existing indexing engine, thereby avoiding recomputation.
Performance example:
Not sure how to test this, as the relevant attributes are in cython code, but I don't think we do tests for indexing engines currently?