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

Impl Extend for LiteMap and avoid quadratic behavior in from_iter and deserialize #6132

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

arthurprs
Copy link
Contributor

@arthurprs arthurprs commented Feb 15, 2025

Adds lm_extend to the StoreMut trait. The Vec implementation is optimized to have O(N) performance on sorted inputs and an asymptotic worst case of O(NLog(N)), effectively avoiding quadratic worst cases.

This PR takes advantage of the new extend optimization and changes the FromIterator and Deserialize implementations to also avoid quadratic worst-case.

Before

litemap/from_iter_rand/small
                        time:   [916.60 ns 941.77 ns 965.82 ns]
litemap/from_iter_rand/large
                        time:   [2.3779 s 2.3863 s 2.3946 s]
litemap/from_iter_sorted/small
                        time:   [84.010 ns 84.084 ns 84.155 ns]
litemap/from_iter_sorted/large
                        time:   [725.31 µs 739.88 µs 755.20 µs]

After

litemap/from_iter_rand/small
                        time:   [814.95 ns 830.50 ns 845.94 ns]
litemap/from_iter_rand/large
                        time:   [42.048 ms 43.138 ms 44.337 ms]
litemap/from_iter_sorted/small
                        time:   [76.707 ns 76.759 ns 76.808 ns]
litemap/from_iter_sorted/large
                        time:   [640.42 µs 652.98 µs 668.38 µs]
litemap/extend_rand/large
                        time:   [108.48 ms 110.88 ms 113.52 ms]
litemap/extend_rand_dups/large
                        time:   [173.80 ms 175.58 ms 177.54 ms]
litemap/extend_from_litemap_rand/large
                        time:   [2.0401 s 2.0488 s 2.0575 s]

This is a followup of #6068

@arthurprs arthurprs requested review from Manishearth, sffc and a team as code owners February 15, 2025 22:25
@arthurprs arthurprs force-pushed the litemap-extend-and-quadratic-worst-case branch 3 times, most recently from 82433f8 to 1f7e87a Compare February 15, 2025 23:02
@arthurprs arthurprs force-pushed the litemap-extend-and-quadratic-worst-case branch from 1f7e87a to 8bd4d3e Compare February 16, 2025 00:09
// Note: this effectively selection sorts the map,
// which isn't efficient for large maps
map.insert(key, value);
out_of_order.push((key, value));
Copy link
Contributor Author

@arthurprs arthurprs Feb 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I considered making this based on StoreMut::lm_push and Store::lm_into_iter by adding the StoreIntoIterator bound to R, but that would be a breaking change.

@@ -43,7 +44,7 @@ criterion = { workspace = true }
default = ["alloc"]
alloc = []
databake = ["dep:databake"]
serde = ["dep:serde"]
serde = ["dep:serde", "alloc"]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for reviewers: The addition of alloc was theoretically a breaking change for people using "no default features" so we may want to bump litemap to 0.8.

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

Successfully merging this pull request may close these issues.

1 participant