Optimize Vec::insert for the case where index == len. by nnethercote · Pull Request #98755 · rust-lang/rust (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation9 Commits1 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

nnethercote

By skipping the call to copy with a zero length. This makes it closer
to push.

I did this recently for SmallVec
(servo/rust-smallvec#282) and it was a big perf win in
one case. Although I don't have a specific use case in mind, it seems
worth doing it for Vec as well.

Things to note:

r? @cuviper

@nnethercote

By skipping the call to copy with a zero length. This makes it closer to push.

I did this recently for SmallVec (servo/rust-smallvec#282) and it was a big perf win in one case. Although I don't have a specific use case in mind, it seems worth doing it for Vec as well.

Things to note:

@rustbot rustbot added the T-libs

Relevant to the library team, which will review and decide on the PR/issue.

label

Jul 1, 2022

@rustbot

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

@joshtriplett

LGTM; not r+ing because you asked for a specific reviewer.

@cuviper

OK -- let's merge alone for perf, but I'm not sure if it's used anywhere in the compiler anyway.

@bors r+ rollup=never

@bors

📌 Commit 679c5ee has been approved by cuviper

@bors bors added S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

and removed S-waiting-on-review

Status: Awaiting review from the assignee but also interested parties.

labels

Jul 1, 2022

@bors

@bors

@rust-timer

Finished benchmarking commit (f99f9e4): comparison url.

Instruction count

mean1 max count2
Regressions 😿 (primary) N/A N/A 0
Regressions 😿 (secondary) N/A N/A 0
Improvements 🎉 (primary) -0.4% -1.0% 8
Improvements 🎉 (secondary) -1.2% -2.4% 13
All 😿🎉 (primary) -0.4% -1.0% 8

Max RSS (memory usage)

Results

mean1 max count2
Regressions 😿 (primary) N/A N/A 0
Regressions 😿 (secondary) N/A N/A 0
Improvements 🎉 (primary) -3.1% -3.1% 1
Improvements 🎉 (secondary) N/A N/A 0
All 😿🎉 (primary) -3.1% -3.1% 1

Cycles

Results

mean1 max count2
Regressions 😿 (primary) 3.6% 3.6% 1
Regressions 😿 (secondary) 3.9% 3.9% 1
Improvements 🎉 (primary) N/A N/A 0
Improvements 🎉 (secondary) N/A N/A 0
All 😿🎉 (primary) 3.6% 3.6% 1

If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf.

@rustbot label: -perf-regression

Footnotes

  1. the arithmetic mean of the percent change ↩2 ↩3
  2. number of relevant changes ↩2 ↩3

@nnethercote

Wow, we actually got a few speedups on the benchmarks. Nice.

riking

@@ -1393,9 +1390,15 @@ impl<T, A: Allocator> Vec<T, A> {
// The spot to put the new value

Choose a reason for hiding this comment

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

This "infallible" comment is now a lie and needs extra justification.

wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request

Oct 11, 2022

@he32

Pkgsrc changes:

Upstream changes:

Version 1.64.0 (2022-09-22)

Language

Compiler

Libraries

Stabilized APIs

These types were previously stable in std::ffi, but are now also available in core and alloc:

These types were previously stable in std::os::raw, but are now also available in core::ffi and std::ffi:

These APIs are now usable in const contexts:

Cargo

Misc

Compatibility Notes

Internal Changes

These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools.

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request

Nov 16, 2022

@he32

Pkgsrc changes:

Upstream changes:

Version 1.64.0 (2022-09-22)

Language

Compiler

Libraries

Stabilized APIs

These types were previously stable in std::ffi, but are now also available in core and alloc:

These types were previously stable in std::os::raw, but are now also available in core::ffi and std::ffi:

These APIs are now usable in const contexts:

Cargo

Misc

Compatibility Notes

Internal Changes

These changes do not affect any public interfaces of Rust, but they represent significant improvements to the performance or internals of rustc and related tools.

Labels

merged-by-bors

This PR was explicitly merged by bors.

S-waiting-on-bors

Status: Waiting on bors to run and complete tests. Bors will change the label on completion.

T-libs

Relevant to the library team, which will review and decide on the PR/issue.