Implement SystemParam for SmallVec

[Shatur]

Objective

To register a system from a script, I need to support for dynamic number of system parameters (since scripting languages usually don't play well with generics). Right now the only solution is to use Vec<T> as a system param which is allocated every time the system runs. But it would be nice to avoid allocations when the number of system parameters is low.

Solution

Implement SystemParam for SmallVec. It's already a dependency for bevy_ecs and allows to avoid allocations with user-defined number of system parameters.

Testing

• Did you test these changes? If so, how? I tried creating a system with a SmallVec<[Local<usize>; 8]>.
• Are there any parts that need more testing? I don't think so. The code follows Vec impl.

[Shatur]

Looks like CI failure is unrelated.

[kfc35]

Looks good to me, follows the regular Vec impl except for one thing: the unsafe impl for ParamSet<'_, '_, SmallVec<[T; N]>>. Is that also necessary to implement? I assume since it works for you, it doesn’t need to be done but just figured I’d ask the question (for my learning)

[Shatur]

Is that also necessary to implement?

I personally don't need it, just implemented it for consistency (since Vec has it.)

[chescock]

Oh, yay, I'm happy to see SystemParamBuilder is getting use!

But it would be nice to avoid allocations when the number of system parameters is low.

Hmm, I wonder whether we could even make a Vec-like parameter that avoids per-run allocations entirely by re-using a buffer between runs.

[chescock]

I think you also need an impl<...> SystemParamBuilder<SmallVec<[P; N]>> for SmallVec<[B; N]> in builder.rs so that you can actually configure the systems.

[Shatur]

Ah, right, I didn't test the builder yet 😅 Added!

[chescock]

It looks like you copied the impl block for ParamSet<Vec<_>>, but not the impl SystemParam, so there's no way to actually obtain a ParamSet<SmallVec>.

I don't think you need a SmallVec version of ParamSet, though. ParamSet<Vec> never actually allocates a Vec, and just creates the subparameters on-demand in get_mut, so there is no allocation to save. So I'd be inclined to delete this impl block.

[Shatur]

Makes sense, removed!

[Shatur]

Hmm, I wonder whether we could even make a Vec-like parameter that avoids per-run allocations entirely by re-using a buffer between runs.

This was my original goal, but I couldn't find where to store the Vec, so I went with SmallVec. If you have any idea, I'm happy to try.

[chescock]

Hmm, I wonder whether we could even make a Vec-like parameter that avoids per-run allocations entirely by re-using a buffer between runs.

This was my original goal, but I couldn't find where to store the Vec, so I went with SmallVec. If you have any idea, I'm happy to try.

I did a few experiments, but without much success.

The "buffer reuse" technique from Wild Performance Tricks seems like it should work, but if you try to put a Vec<T::Item<'static, 'static>> in the state then the compiler complains that it can't prove Self::Item::State == Self::State because it can't prove T::Item<'w, 's>::Item<'static, 'static> == T::Item<'static, 'static>.

Then I thought that a Vec that dropped its contents but didn't deallocate sounded like bumpalo::collections::Vec, which we already have a dependency on! But the Item type would be Vec<'state, T::Item<'world, 'state>> and that's invalid because it requires T::Item<'world, 'state>: 'state, which doesn't hold.

I think I might have gotten it working using raw pointers! You can use alloc::alloc() to manually allocate a buffer and hold the raw pointer in the State. It's a lot of unsafe, though.

[Shatur]

I think I might have gotten it working using raw pointers! You can use alloc::alloc() to manually allocate a buffer and hold the raw pointer in the State. It's a lot of unsafe, though.

Would be interesting to see!