Skip to content

/ rust-smallvec

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

bench and optimize `from_slice` #118

Merged
merged 1 commit into from Aug 22, 2018
Merged

bench and optimize `from_slice` #118

merged 1 commit into from Aug 22, 2018

Conversation

@llogiq
Copy link
Contributor

llogiq commented Aug 22, 2018 •
edited by larsbergstrom

This adds "bench_from_slice(_vec)(_small)" benches, renaming the previous versions to "bench_from_iter*" (because they were using From<&[T]>, which calls FromIterator internally) and optimizes the from_slice constructor considerably.

Before:

test bench_from_slice                  ... bench:          42 ns/iter (+/- 0)
test bench_from_slice_small            ... bench:          12 ns/iter (+/- 0)

After:

test bench_from_slice                  ... bench:          27 ns/iter (+/- 0)
test bench_from_slice_small            ... bench:           8 ns/iter (+/- 0)

This change is Reviewable
@llogiq
Copy link
Contributor Author

llogiq commented Aug 22, 2018

travis error is unrelated – current nightly fails to build master as well.
@mbrubeck
Copy link
Contributor

mbrubeck commented Aug 22, 2018

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Aug 22, 2018

📌 Commit 2894fc5 has been approved by mbrubeck
bors-servo added a commit that referenced this pull request Aug 22, 2018
@bors-servo
Auto merge of #118 - llogiq:optimize-from-slice, r=mbrubeck
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
2aa599d 
bench and optimize `from_slice`

This adds "bench_from_slice(_vec)(_small)" benches, renaming the previous versions to "bench_from_iter*" (because they were using `From<&[T]>`, which calls `FromIterator` internally) and optimizes the `from_slice` constructor considerably.

Before:
```
test bench_from_slice                  ... bench:          42 ns/iter (+/- 0)
test bench_from_slice_small            ... bench:          12 ns/iter (+/- 0)
```
After:
```
test bench_from_slice                  ... bench:          27 ns/iter (+/- 0)
test bench_from_slice_small            ... bench:           8 ns/iter (+/- 0)
```

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-smallvec/118)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Aug 22, 2018

⌛ Testing commit 2894fc5 with merge 2aa599d...
@bors-servo
Copy link
Contributor

bors-servo commented Aug 22, 2018

💔 Test failed - status-travis
@mbrubeck
Copy link
Contributor

mbrubeck commented Aug 22, 2018

Looks like this needs a #[cfg(not(feature = "std"))] use alloc::borrow::ToOwned;.
@llogiq llogiq force-pushed the llogiq:optimize-from-slice branch from 2894fc5 to b0c7dd1 Aug 22, 2018
@llogiq
Copy link
Contributor Author

llogiq commented Aug 22, 2018

I'll settle for .to_vec() instead.
@llogiq llogiq force-pushed the llogiq:optimize-from-slice branch from b0c7dd1 to 0801880 Aug 22, 2018
@llogiq
bench and optimize `from_slice`
Loading status checks…
7633aeb 
This adds "bench_from_slice(_vec)(_small)" benches, renaming the previous
versions to "bench_from_iter*" (because they were using `From<&[T]>`, which
calls `FromIterator` internally) and optimizes the `from_slice` constructor
considerably.

Before:
```
test bench_from_slice                  ... bench:          42 ns/iter (+/- 0)
test bench_from_slice_small            ... bench:          12 ns/iter (+/- 0)
```
After:
```
test bench_from_slice                  ... bench:          27 ns/iter (+/- 0)
test bench_from_slice_small            ... bench:           8 ns/iter (+/- 0)
```
@llogiq llogiq force-pushed the llogiq:optimize-from-slice branch from 0801880 to 7633aeb Aug 22, 2018
@llogiq
Copy link
Contributor Author

llogiq commented Aug 22, 2018 •
edited

@mbrubeck this is ready to merge now.

Also perhaps we could get a new version out now that rustc is going to use it...
@mbrubeck
Copy link
Contributor

mbrubeck commented Aug 22, 2018

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Aug 22, 2018

📌 Commit 7633aeb has been approved by mbrubeck
@bors-servo
Copy link
Contributor

bors-servo commented Aug 22, 2018

⌛ Testing commit 7633aeb with merge dc6c0ac...
bors-servo added a commit that referenced this pull request Aug 22, 2018
@bors-servo
Auto merge of #118 - llogiq:optimize-from-slice, r=mbrubeck
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
dc6c0ac 
bench and optimize `from_slice`

This adds "bench_from_slice(_vec)(_small)" benches, renaming the previous versions to "bench_from_iter*" (because they were using `From<&[T]>`, which calls `FromIterator` internally) and optimizes the `from_slice` constructor considerably.

Before:
```
test bench_from_slice                  ... bench:          42 ns/iter (+/- 0)
test bench_from_slice_small            ... bench:          12 ns/iter (+/- 0)
```
After:
```
test bench_from_slice                  ... bench:          27 ns/iter (+/- 0)
test bench_from_slice_small            ... bench:           8 ns/iter (+/- 0)
```

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-smallvec/118)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Aug 22, 2018

☀️ Test successful - status-travis
Approved by: mbrubeck
Pushing dc6c0ac to master...
@bors-servo bors-servo merged commit 7633aeb into servo:master Aug 22, 2018
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
mbrubeck added a commit to mbrubeck/rust-smallvec that referenced this pull request Aug 22, 2018
@mbrubeck
Version 0.6.5
a95b2d0 
Change log:

* servo#118 - optimize `from_slice`
* servo#115 - add `into_inner` method
* Some code cleanup and testing improvements (servo#112, servo#113, servo#114, servo#120)
mbrubeck added a commit to mbrubeck/rust-smallvec that referenced this pull request Aug 22, 2018
@mbrubeck
Version 0.6.5
840426b 
Change log:

* servo#115 - add `into_inner` method
* servo#117 - add `from_buf_and_len` and `from_buf_and_len_unchecked`
* servo#118 - optimize `from_slice`
* Some code cleanup and testing improvements (servo#112, servo#113, servo#114, servo#120)
@mbrubeck mbrubeck mentioned this pull request Aug 22, 2018
Merged
bors-servo added a commit that referenced this pull request Aug 22, 2018
@bors-servo
Auto merge of #121 - mbrubeck:bump, r=Manishearth
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
Loading status checks…
04e3c4e 
Version 0.6.5

Change log:

* #115 - add `into_inner` method
* #117 - add `from_buf_and_len` and `from_buf_and_len_unchecked`
* #118 - optimize `from_slice`
* Some code cleanup and testing improvements (#112, #113, #114, #120)

cc @llogiq

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/rust-smallvec/121)
<!-- Reviewable:end -->
@llogiq llogiq deleted the llogiq:optimize-from-slice branch Aug 23, 2018
@arthurprs

This comment has been minimized.

Sign in to view
Copy link
Contributor

arthurprs commented on lib.rs in 7633aeb Aug 23, 2018 •
edited

Should this be b.capacity()? I'm gessing to_vec may over allocate.

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

llogiq replied Aug 23, 2018

Good catch! I had .to_owned there initially, which doesn't, and failed to check. I'll send a followup PR.

This comment has been minimized.

Sign in to view
Copy link
Contributor

mbrubeck replied Aug 23, 2018

Currently slice::to_vec never over-allocates, so this is safe in current versions of Rust, but it's still worth fixing in case that behavior doesn't hold in future versions.
@mbrubeck mbrubeck mentioned this pull request Aug 23, 2018
Merged
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
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
@llogiq @mbrubeck @bors-servo @arthurprs
You can’t perform that action at this time.