-
Notifications
You must be signed in to change notification settings - Fork 891
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
Improve memory footprint of isin by using contains #14478
Conversation
Previously, isin was implemented using an inner join between the column we are searching (the haystack) and the values we are searching for (the needles). This had a large memory footprint when there were repeated needles (since that blows up the cardinality of the merge). To fix this, note that we don't need to do a merge at all, since libcudf provides a primitive (contains) to search for many needles in a haystack. The only thing we must bear in mind is that left.isin(right) is asking for the locations in left that match an entry in right, whereas contains(haystack, needles) provides a bool mask that selects needles that are in the haystack. To get the behaviour we want, we therefore need to do contains(right, left) and treat the values to search for as the haystack. As well as having a much better memory footprint, this hash-based approach search is significantly faster than the previous merge-based one. While we are here, lower the memory footprint of MultiIndex.isin by using a left-semi join (the implementation is separate from the isin implementation on columns and looks a little more complicated to unpick). - Closes rapidsai#14298
09939f0
to
7848147
Compare
Using the data from #14298: Old:
New
Happy thanksgiving @thomcom |
Hmph, broken somehow... |
The problem is that |
The result returned from libcudf is masked by the null mask of the needles. If it has any nulls we must replace them with whether or not the haystack contains nulls to match the semantics we need for isin.
cef017e
to
884f9ce
Compare
/merge |
Description
Previously, isin was implemented using an inner join between the column we are searching (the haystack) and the values we are searching for (the needles). This had a large memory footprint when there were repeated needles (since that blows up the cardinality of the merge).
To fix this, note that we don't need to do a merge at all, since libcudf provides a primitive (contains) to search for many needles in a haystack. The only thing we must bear in mind is that left.isin(right) is asking for the locations in left that match an entry in right, whereas contains(haystack, needles) provides a bool mask that selects needles that are in the haystack. To get the behaviour we want, we therefore need to do contains(right, left) and treat the values to search for as the haystack.
As well as having a much better memory footprint, this hash-based approach search is significantly faster than the previous merge-based one.
While we are here, lower the memory footprint of MultiIndex.isin by using a left-semi join (the implementation is separate from the isin implementation on columns and looks a little more complicated to unpick).
Checklist