Skip to content

Shrink code produced by smallvec![]. #385

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

Merged
merged 1 commit into from
Jun 5, 2025
Merged

Conversation

nnethercote
Copy link
Contributor

Currently smallvec![] expands to this:

{
    let count = 0usize;
    #[allow(unused_mut)]
    let mut vec = ::smallvec::SmallVec::new();
    if count <= vec.capacity() {
	vec
    } else {
	::smallvec::SmallVec::from_vec(::alloc::vec::Vec::new())
    }
};

This commit adds a rule to the smallvec! macro for the zero-length case so it instead expands to this:

::smallvec::SmallVec::new()

The std::vec! macro already has a similar special case.

This commit also improves the non-zero case.

  • It removes the #[allow(unused_mut)], which was only needed for the zero-length case.
  • It changes the * repetitions to +. (Again, like std::vec.)

Currently `smallvec![]` expands to this:
```
{
    let count = 0usize;
    #[allow(unused_mut)]
    let mut vec = ::smallvec::SmallVec::new();
    if count <= vec.capacity() {
	vec
    } else {
	::smallvec::SmallVec::from_vec(::alloc::vec::Vec::new())
    }
};
```
This commit adds a rule to the `smallvec!` macro for the zero-length
case so it instead expands to this:
```
::smallvec::SmallVec::new()
```
The `std::vec!` macro already has a similar special case.

This commit also improves the non-zero case.
- It removes the `#[allow(unused_mut)]`, which was only needed for the
  zero-length case.
- It changes the `*` repetitions to `+`. (Again, like `std::vec`.)
@nnethercote
Copy link
Contributor Author

This helps with compile times. I wrote a stress test with 5,000 smallvec![] calls, like this:

use smallvec::{SmallVec, smallvec};

fn main() {
    type S = SmallVec<u32, 2>;
    let _: S = smallvec![];
    let _: S = smallvec![];
    let _: S = smallvec![];
    // ... 4997 more repetitions ...
}

And the compile time improved dramatically:

old new
check 9.6s 0.5s
build 11.3s 0.8s

Copy link
Collaborator

@mbrubeck mbrubeck left a comment

Choose a reason for hiding this comment

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

Thanks!

@mbrubeck mbrubeck added this pull request to the merge queue Jun 5, 2025
Merged via the queue into servo:v2 with commit 71c8abb Jun 5, 2025
6 checks passed
@nnethercote
Copy link
Contributor Author

@mbrubeck: is it worth backporting this to 1.x? I'm not sure what the status of 1.x is, whether future releases are likely, etc.

@nnethercote nnethercote deleted the smallvec-0 branch June 5, 2025 08:53
@mbrubeck
Copy link
Collaborator

mbrubeck commented Jun 5, 2025

Yes, I think this is worth backporting. 1.x will probably be used more than 2.x for quite a while yet.

@nnethercote
Copy link
Contributor Author

Ok. What's the process? Is it something you can do, or should I do it? If the latter, what do I do? Just make a PR against the v1 branch? I've never made a PR against the non-default branch but I guess there's a way to do it.

@nicoburns
Copy link
Contributor

I've never made a PR against the non-default branch but I guess there's a way to do it.

You change the "base" in the PR creation UI:

Screenshot 2025-06-06 at 00 28 21

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.

3 participants