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

[C++] Optimize Take implementation #22185

Closed
asfimport opened this issue Jun 27, 2019 · 3 comments
Closed

[C++] Optimize Take implementation #22185

asfimport opened this issue Jun 27, 2019 · 3 comments

Comments

@asfimport
Copy link
Collaborator

asfimport commented Jun 27, 2019

There is some question of whether these kernels allocate optimally- for example when Filtering or Taking strings it might be more efficient to pass over the filter/indices twice, first to determine how much character storage will be needed then again into allocated memory: #4531 (comment)

Additionally, these kernels could probably make good use of scatter/gather SIMD instructions.

Furthermore, Filter's bitmap is currently lazily expanded into the indices of elements to be appended to the output array. It would probably be more efficient to expand to indices in batches, then gather using an index batch.

Reporter: Ben Kietzman / @bkietz
Assignee: Wes McKinney / @wesm

Related issues:

PRs and other links:

Note: This issue was originally created as ARROW-5760. Please see the migration documentation for further details.

@asfimport
Copy link
Collaborator Author

Wes McKinney / @wesm:
Another problem I noticed with the current implementation of Take and Filter: different x86 is generated for applying these operations on arrays with the same underlying C type. For example, instructions for moving 8-byte-wide values are being generated for Int64Type, UInt64Type, Date64Type, Time64Type, and TimestampType, when only one underlying "data movement function" is needed. As part of improving the performance of Take and Filter we should also ensure that we eliminate this unneeded binary bloat in the shared library

@asfimport
Copy link
Collaborator Author

Wes McKinney / @wesm:
I'd like to work on this next week if it's alright

@asfimport
Copy link
Collaborator Author

Antoine Pitrou / @pitrou:
Issue resolved by pull request 7382
#7382

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants