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

chore(performance): Enable the "union" feature on SmallVec #9772

Merged
merged 1 commit into from
Oct 22, 2021

Conversation

bruceg
Copy link
Member

@bruceg bruceg commented Oct 22, 2021

This feature is not enabled by default. From the docs: When the union
feature is enabled smallvec will track its state (inline or spilled)
without the use of an enum tag, reducing the size of the smallvec by one
machine word.

Signed-off-by: Bruce Guenter bruce.guenter@datadoghq.com

This feature is not enabled by default.  From the docs: When the union
feature is enabled smallvec will track its state (inline or spilled)
without the use of an enum tag, reducing the size of the smallvec by one
machine word.

Signed-off-by: Bruce Guenter <bruce.guenter@datadoghq.com>
@bruceg bruceg added type: tech debt A code change that does not add user value. domain: performance Anything related to Vector's performance labels Oct 22, 2021
@bruceg bruceg requested review from blt and tobz October 22, 2021 20:29
@netlify
Copy link

netlify bot commented Oct 22, 2021

✔️ Deploy Preview for vector-project ready!

🔨 Explore the source changes: bade85e

🔍 Inspect the deploy log: https://app.netlify.com/sites/vector-project/deploys/61731f10d6f6a80008379dd4

😎 Browse the preview: https://deploy-preview-9772--vector-project.netlify.app

@jszwedko
Copy link
Member

@bruceg do you know why they don't enable it by default? I'm wondering what the trade-off is.

@blt
Copy link
Contributor

blt commented Oct 22, 2021

@bruceg do you know why they don't enable it by default? I'm wondering what the trade-off is.

I believe the only downside is it requires rust 1.49.

@bruceg
Copy link
Member Author

bruceg commented Oct 22, 2021

Using the union would likely lead to (slightly?) more complex code. According to the initial PR for the union feature servo/rust-smallvec#94 there is a performance hit in one of their tests, though most show better performance. Other than that, I don't know

@bruceg bruceg merged commit b71029c into master Oct 22, 2021
@bruceg bruceg deleted the smallvec-union branch October 22, 2021 23:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain: performance Anything related to Vector's performance type: tech debt A code change that does not add user value.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants