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.

Sign up for GitHub

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

Jump to bottom

libcollection : Switches from uint to u32 in BitV and BitVSet #18018

Merged
merged 1 commit into from
Oct 15, 2014
Merged

libcollection : Switches from uint to u32 in BitV and BitVSet #18018

merged 1 commit into from
Oct 15, 2014

Conversation

gamazeps
Copy link
Contributor

Closes #16736
linked to #18009

@Gankra
Copy link
Contributor

Gankra commented Oct 13, 2014

Did you run the benches (PLEASE_BENCH=1) on this? I'd be interested to see if there were perf consequences on 64-bit platforms.

CC @apoelstra

@gamazeps
Copy link
Contributor Author

Yup, here are the result with u32

test bitv::tests::bench_bitv_big_iter ... bench: 17185 ns/iter (+/- 634)
test bitv::tests::bench_bitv_big_union ... bench: 1017 ns/iter (+/- 22)
test bitv::tests::bench_bitv_set_big_fixed ... bench: 681 ns/iter (+/- 5)
test bitv::tests::bench_bitv_set_big_variable ... bench: 1694 ns/iter (+/- 25)
test bitv::tests::bench_bitv_set_small ... bench: 643 ns/iter (+/- 11)
test bitv::tests::bench_bitv_small_iter ... bench: 474 ns/iter (+/- 23)
test bitv::tests::bench_bitvset_big ... bench: 993 ns/iter (+/- 5)
test bitv::tests::bench_bitvset_iter ... bench: 25473 ns/iter (+/- 726)
test bitv::tests::bench_bitvset_small ... bench: 989 ns/iter (+/- 14)
test bitv::tests::bench_uint_small ... bench: 572 ns/iter (+/- 7)

and the results with uint
test bitv::tests::bench_bitv_big_iter ... bench: 17347 ns/iter (+/- 650)
test bitv::tests::bench_bitv_big_union ... bench: 519 ns/iter (+/- 8)
test bitv::tests::bench_bitv_set_big_fixed ... bench: 685 ns/iter (+/- 10)
test bitv::tests::bench_bitv_set_big_variable ... bench: 1713 ns/iter (+/- 26)
test bitv::tests::bench_bitv_set_small ... bench: 674 ns/iter (+/- 11)
test bitv::tests::bench_bitv_small_iter ... bench: 782 ns/iter (+/- 22)
test bitv::tests::bench_bitvset_big ... bench: 1069 ns/iter (+/- 10)
test bitv::tests::bench_bitvset_iter ... bench: 40832 ns/iter (+/- 7077)
test bitv::tests::bench_bitvset_small ... bench: 1065 ns/iter (+/- 30)
test bitv::tests::bench_uint_small ... bench: 602 ns/iter (+/- 5)

u32 are faster for everything but unions, for which they are much slower.
I think it's a good tradeoff, what's your opinion on it ?

cc @thestinger @aturon

@Gankra
Copy link
Contributor

Gankra commented Oct 14, 2014

Looks good to me! Thanks a bunch!

@gamazeps
Copy link
Contributor Author

r? @aturon

bors added a commit that referenced this pull request Oct 14, 2014
@bors
auto merge of #18018 : gamazeps/rust/isuue16736, r=cmr
40b2449 
Closes #16736
linked to #18009
@bors bors closed this Oct 15, 2014
@bors bors merged commit 1ef5e38 into rust-lang:master Oct 15, 2014
@gamazeps gamazeps mentioned this pull request Oct 16, 2014
Closed
bors added a commit that referenced this pull request Oct 17, 2014
@bors
auto merge of #18089 : gamazeps/rust/small-bitv-cleanup, r=alexcrichton
93e589c 
I was going to write some doc in order to remove the #[allow(missing_doc)] but there was actually none missing.
I also removed a warning i didn't see in my last commit  #18018
Linked to #18009
@gamazeps gamazeps deleted the isuue16736 branch October 17, 2014 11:43
@l0kod l0kod mentioned this pull request Oct 18, 2014
Closed
lnicola pushed a commit to lnicola/rust that referenced this pull request Sep 25, 2024
@bors
Auto merge of rust-lang#18018 - ChayimFriedman2:unit-ret-complete-sem…
97907ca 
…i, r=Veykril

feat: Automatically add semicolon when completing unit-returning functions

But provide a config to suppress that.

I didn't check whether we are in statement expression position, because this is hard in completion (due to the natural incompleteness of source code when completion is invoked), and anyway using function returning unit as an argument to something seems... dubious.

Fixes rust-lang#17263.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Bitv uses architecture-sized uints for backing storage
4 participants
@gamazeps @Gankra @emberian @bors