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.

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

Avoid heap allocation for initial value of vector longhands which do not allow empty #15807

Open
upsuper opened this issue Mar 3, 2017 · 6 comments
Assignees
Labels

Comments

@upsuper
Copy link
Member

@upsuper upsuper commented Mar 3, 2017

So far there are various vector longhands which have allow_empty being false. The initial value of those properties would always need a heap allocation for the only element in the vector. This should be fixed.

Gecko avoids this via having the first element stored inline. Servo can probably use this method as well.

cc @emilio @Manishearth

@Manishearth
Copy link
Member

@Manishearth Manishearth commented Mar 3, 2017

These should use Option<Box<T>> (or Either<None, Box<T>>) explicitly.

@upsuper
Copy link
Member Author

@upsuper upsuper commented Mar 3, 2017

That probably makes things more complicated. It should just be

struct AutoVec<T> {
  first: T,
  rest: Vec<T>,
}

and you can implement index trait on it.

@bholley bholley added the A-stylo label Mar 26, 2017
@highfive
Copy link

@highfive highfive commented Mar 26, 2017

cc @emilio

@bholley
Copy link
Contributor

@bholley bholley commented Mar 26, 2017

We have SmallVec for this purpose.

@nox
Copy link
Member

@nox nox commented Jun 27, 2017

I'm working on things that are similar to this.

I think we should go for:

enum ToBeNamed<T> {
    Single(T),
    Many(Box<[T]>),
}

This is smaller than SmallVec because we don't need to care about any space wasted for the vector capacity or the number of inlined items.

@nox nox self-assigned this Jun 27, 2017
@nox
Copy link
Member

@nox nox commented Jun 27, 2017

In the opposite direction, I just made this for properties that can be empty and have an empty initial value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.