Skip to content

RFC: Btree query API #27135

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

Closed
wants to merge 3 commits into from
Closed

RFC: Btree query API #27135

wants to merge 3 commits into from

Conversation

Gankra
Copy link
Contributor

@Gankra Gankra commented Jul 19, 2015

This is an implementation of the core functionality of rust-lang/rfcs#1195 which is not yet accepted (largely due to bikeshedding naming -- but also on what removal/entry APIs to expose).

Note that the vast majority of the code was generated by a program (see comments).

I believe I've exhaustively tested every single code path with my unit test.

Note that removal/entry APIs are not yet provided; I'm of the opinion that a major refactor of the internals to use parent pointers should be done first. Then search/removal/entry/range can basically share the exact same source code with no overhead. It's also expected to be substantially more efficient due to not needing to maintain an explicit search stack for insert/remove/range/entry.

@rust-highfive
Copy link
Contributor

r? @nikomatsakis

(rust_highfive has picked a reviewer for you, use r? to override)

@Gankra
Copy link
Contributor Author

Gankra commented Jul 19, 2015

cc @apasel422 @gereeter

@apasel422
Copy link
Contributor

@gankro Wow, this is really cool, but definitely highlights the need for some kind of parametric mutability.

self.map.max().map(|r| r.0)
}

/// Gets the closest value to the given key that is less than it.
Copy link
Contributor

Choose a reason for hiding this comment

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

s/key/value/

(Same for the following methods.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

All should presumably be element.

@Gankra
Copy link
Contributor Author

Gankra commented Jul 19, 2015

I'm just really happy I didn't have to write 12 node search procedures as well. I was dreading the impl until I realized Found/GoDown was basically sufficient.

}};
}},
Internal(internal_handle) => {{
temp_node = internal_handle.into_{dir}_edge{muta_ext}().into_edge{muta_ext}();
Copy link
Contributor

Choose a reason for hiding this comment

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

There was a tidy error on this line.

@Gankra
Copy link
Contributor Author

Gankra commented Jul 20, 2015

Bench results:

get_rand_10000_eq                       ... bench:          50 ns/iter (+/- 7)
get_rand_10000_ge                       ... bench:          53 ns/iter (+/- 2)
get_rand_10000_ge_range                 ... bench:         117 ns/iter (+/- 7)
get_rand_10000_gt                       ... bench:          56 ns/iter (+/- 6)
get_rand_10000_gt_range                 ... bench:         112 ns/iter (+/- 6)
get_rand_10000_le                       ... bench:          53 ns/iter (+/- 4)
get_rand_10000_le_range                 ... bench:         110 ns/iter (+/- 7)
get_rand_10000_lt                       ... bench:          58 ns/iter (+/- 5)
get_rand_10000_lt_range                 ... bench:         115 ns/iter (+/- 11)

get_rand_100_eq                         ... bench:          10 ns/iter (+/- 2)
get_rand_100_ge                         ... bench:          11 ns/iter (+/- 2)
get_rand_100_ge_range                   ... bench:          60 ns/iter (+/- 8)
get_rand_100_gt                         ... bench:          12 ns/iter (+/- 2)
get_rand_100_gt_range                   ... bench:          58 ns/iter (+/- 15)
get_rand_100_le                         ... bench:          11 ns/iter (+/- 2)
get_rand_100_le_range                   ... bench:          57 ns/iter (+/- 2)
get_rand_100_lt                         ... bench:          14 ns/iter (+/- 2)
get_rand_100_lt_range                   ... bench:          63 ns/iter (+/- 20)

get_seq_10000_eq                        ... bench:          35 ns/iter (+/- 5)
get_seq_10000_ge                        ... bench:          40 ns/iter (+/- 5)
get_seq_10000_ge_range                  ... bench:          93 ns/iter (+/- 7)
get_seq_10000_gt                        ... bench:          40 ns/iter (+/- 2)
get_seq_10000_gt_range                  ... bench:          98 ns/iter (+/- 5)
get_seq_10000_le                        ... bench:          35 ns/iter (+/- 2)
get_seq_10000_le_range                  ... bench:          97 ns/iter (+/- 25)
get_seq_10000_lt                        ... bench:          41 ns/iter (+/- 3)
get_seq_10000_lt_range                  ... bench:         103 ns/iter (+/- 9)

get_seq_100_eq                          ... bench:          11 ns/iter (+/- 1)
get_seq_100_ge                          ... bench:          12 ns/iter (+/- 0)
get_seq_100_ge_range                    ... bench:          64 ns/iter (+/- 9)
get_seq_100_gt                          ... bench:          13 ns/iter (+/- 3)
get_seq_100_gt_range                    ... bench:          64 ns/iter (+/- 8)
get_seq_100_le                          ... bench:          11 ns/iter (+/- 2)
get_seq_100_le_range                    ... bench:          63 ns/iter (+/- 153)
get_seq_100_lt                          ... bench:          15 ns/iter (+/- 1)
get_seq_100_lt_range                    ... bench:          68 ns/iter (+/- 18)

insert_rand_100                         ... bench:         105 ns/iter (+/- 11)
insert_rand_10000                       ... bench:         138 ns/iter (+/- 14)
insert_seq_100                          ... bench:         107 ns/iter (+/- 13)
insert_seq_10000                        ... bench:         201 ns/iter (+/- 36)

iter_100000_plain                       ... bench:     891,492 ns/iter (+/- 93,913)
iter_100000_with_queries_backward       ... bench:   4,648,407 ns/iter (+/- 690,988)
iter_100000_with_queries_forward        ... bench:   4,580,392 ns/iter (+/- 760,890)

iter_1000_plain                         ... bench:       8,500 ns/iter (+/- 1,250)
iter_1000_with_queries_backward         ... bench:      31,266 ns/iter (+/- 2,909)
iter_1000_with_queries_forward          ... bench:      29,045 ns/iter (+/- 2,350)

iter_20_plain                           ... bench:         170 ns/iter (+/- 19)
iter_20_with_queries_backward           ... bench:         207 ns/iter (+/- 35)
iter_20_with_queries_forward            ... bench:         159 ns/iter (+/- 38)

@@ -228,6 +228,665 @@ impl<K: Ord, V> BTreeMap<K, V> {
}
}

// Rust's macro system is so attrocious at being generic over mutability
Copy link
Contributor

Choose a reason for hiding this comment

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

s/attrocious/atrocious/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This correction made my day

@Gankra
Copy link
Contributor Author

Gankra commented Aug 12, 2015

RFC closed.

@Gankra Gankra closed this Aug 12, 2015
@Gankra
Copy link
Contributor Author

Gankra commented Aug 12, 2015

@apasel422 Note that you might want to steal these bench changes for profiling parent pointer stuff.

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.

4 participants