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

Discussion on the usage of SmallVec #9

Open
GabrielMajeri opened this issue Jun 22, 2018 · 1 comment
Open

Discussion on the usage of SmallVec #9

GabrielMajeri opened this issue Jun 22, 2018 · 1 comment

Comments

@GabrielMajeri
Copy link
Contributor

Voodoo uses SmallVec in lots of places. Besides making the API more complicated, I doubt it improves performance.

From this blog post by a smallvec developer:

Because malloc is fast, for many cases it’s actually slower to use SmallVec than just using Vec because the one-time cost of the initial allocation is dwarfed by the lifetime cost of SmallVec’s increased complexity. You can see that switching to Vec actually improves speed on many of SmallVec’s own benchmarks.

SmallVec is useful in cases where we perform lots of small allocations, which are unlikely to require allocating on the heap. But in Vulkan allocations are done at resource creation time, off the critical path.

Other Vulkan bindings like ash use Vec instead of smallvec (vulkano notwithstanding, it uses it internally, when doing lots of small allocations).


I suggest we switch Voodoo to use the standard Vec in most places, or even Box, considering the users of the API will likely not try to push new stuff into an array returned by Vulkan.

Instead of returning SmallVec<[QueueFamilyProperties; 16]>, we could return Box<[QueueFamilyProperties]>.

@c0gent
Copy link
Member

c0gent commented Jun 22, 2018

Good observations. I haven't actually benchmarked the difference between SmallVec and Vec. It would probably be hard to find much of a difference in a real world program but I'd be open to switching if we had some good evidence it made any difference. Feel free to create some benchmarks and see if you can find anything.

A boxed array would not be an appropriate return type as it has no len component and basically offers no advantages over a Vec. Array access would be easy to mess up and bug prone.

I appreciate your feedback. I encourage you to fork the project and experiment with any changes you feel would be beneficial.

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

No branches or pull requests

2 participants