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

Copy the new Vec::retain implementation into the retain_mut function #87

Closed
Kerollmops opened this issue Feb 19, 2021 · 1 comment
Closed

Comments

@Kerollmops
Copy link
Member

In some previous PRs (#83, #85), we introduced the retain_mut helper function that permits the modification of the visited elements by giving a &mut of the elements. Now that the standard library Vec::retain function was rewritten and gives more than 20% performance improvement, it would be cool to patch our own function too!

@Kerollmops Kerollmops pinned this issue Feb 19, 2021
@upsuper
Copy link

upsuper commented Feb 20, 2021

FWIW, you can use my crate retain_mut. It hasn't updated the code, but I do plan to when the change reaches stable.

(Actually the change to Rust std was motivated by my inspect of the retain_mut crate for which I raised this inefficiency in a Rust group and another member decided to fix it :))

bors bot added a commit that referenced this issue Feb 20, 2021
89: Improve unions between arrays r=Kerollmops a=Kerollmops

I found out that merging two arrays was quite slow while benchmarking our new search engine. Doing unions of array containers needed improvement. I reworked the algorithm to make it easier to read and removed the inserts triggering a lot of useless memory copy, and made it write into a new allocation.

I found out that CRoaring (which is the roaring implementation in c) [was doing a lot SIMD based things to do fast array unions](https://github.com/RoaringBitmap/CRoaring/blob/63a54b15df3bc8fad77a9cc26fbc2dcce5f8da23/src/array_util.c#L1894-L1915), I did not implement them because it would take me a lot of time and that I would prefer using the, soon to be released, [stdsimd library](https://github.com/rust-lang/stdsimd).

90: Use the retain_mut external library r=Kerollmops a=Kerollmops

Related to #87 but do not fix it yet.

Thanks to @upsuper and its [retain_mut library](https://docs.rs/retain_mut), we can remove our internal copy of the [std `retain` function](https://doc.rust-lang.org/std/vec/struct.Vec.html#method.retain) and let it be maintained externally (until `retain_mut` is merged into the std).

Co-authored-by: Clément Renault <[email protected]>
bors bot added a commit that referenced this issue Feb 20, 2021
90: Use the retain_mut external library r=Kerollmops a=Kerollmops

Related to #87 but do not fix it yet.

Thanks to @upsuper and its [retain_mut library](https://docs.rs/retain_mut), we can remove our internal copy of the [std `retain` function](https://doc.rust-lang.org/std/vec/struct.Vec.html#method.retain) and let it be maintained externally (until `retain_mut` is merged into the std).

Co-authored-by: Clément Renault <[email protected]>
bors bot added a commit that referenced this issue Feb 20, 2021
89: Improve unions between arrays r=Kerollmops a=Kerollmops

I found out that merging two arrays was quite slow while benchmarking our new search engine. Doing unions of array containers needed improvement. I reworked the algorithm to make it easier to read and removed the inserts triggering a lot of useless memory copy, and made it write into a new allocation.

I found out that CRoaring (which is the roaring implementation in c) [was doing a lot SIMD based things to do fast array unions](https://github.com/RoaringBitmap/CRoaring/blob/63a54b15df3bc8fad77a9cc26fbc2dcce5f8da23/src/array_util.c#L1894-L1915), I did not implement them because it would take me a lot of time and that I would prefer using the, soon to be released, [stdsimd library](https://github.com/rust-lang/stdsimd).

90: Use the retain_mut external library r=Kerollmops a=Kerollmops

Related to #87 but do not fix it yet.

Thanks to @upsuper and its [retain_mut library](https://docs.rs/retain_mut), we can remove our internal copy of the [std `retain` function](https://doc.rust-lang.org/std/vec/struct.Vec.html#method.retain) and let it be maintained externally (until `retain_mut` is merged into the std).

Co-authored-by: Clément Renault <[email protected]>
@Kerollmops Kerollmops unpinned this issue May 2, 2021
not-jan pushed a commit to not-jan/roaring-rs that referenced this issue Aug 31, 2022
89: Improve unions between arrays r=Kerollmops a=Kerollmops

I found out that merging two arrays was quite slow while benchmarking our new search engine. Doing unions of array containers needed improvement. I reworked the algorithm to make it easier to read and removed the inserts triggering a lot of useless memory copy, and made it write into a new allocation.

I found out that CRoaring (which is the roaring implementation in c) [was doing a lot SIMD based things to do fast array unions](https://github.com/RoaringBitmap/CRoaring/blob/63a54b15df3bc8fad77a9cc26fbc2dcce5f8da23/src/array_util.c#L1894-L1915), I did not implement them because it would take me a lot of time and that I would prefer using the, soon to be released, [stdsimd library](https://github.com/rust-lang/stdsimd).

90: Use the retain_mut external library r=Kerollmops a=Kerollmops

Related to RoaringBitmap#87 but do not fix it yet.

Thanks to @upsuper and its [retain_mut library](https://docs.rs/retain_mut), we can remove our internal copy of the [std `retain` function](https://doc.rust-lang.org/std/vec/struct.Vec.html#method.retain) and let it be maintained externally (until `retain_mut` is merged into the std).

Co-authored-by: Clément Renault <[email protected]>
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