new PooledList<PlayerState>(Players.Count, preallocate : true)

[dzmitry-lahoda]

Is your feature request related to a problem? Please describe. Zero overhead structs clone.

Describe the solution you'd like

            PooledList<PlayerState> players = null;
            if (Players != null)
            {
                players = new PooledList<PlayerState>(Players.Count, preallocate : true); // i guess preallocated just sets _size = capacity;
                for (var i = 0; i < players.Count; i++)
                {
                    PlayerState.Clone(in Players.Span[i], ref players.Span[i]);
                }
            }

Describe alternatives you've considered

            PooledList<PlayerState> players = null;
            if (Players != null)
            {
                players = new PooledList<PlayerState>(Players.Count);
                for (var i = 0; i < players.Count; i++)
                {
                    // Clone will fail until
                    players[i] = default; // overhead to create 200+ byte struct on stack and pass it into indexer (not sure if optimized away by compilers?)
                    PlayerState.Clone(in Players.Span[i], ref players.Span[i]);
                }
            }

Additional context Usual List could not do that because it would leak abstraction (that it is actually array list). PooledList already leaks Span as property and ArrayPool as ctor arg(also ctor arg is no of so problem). So adding that method will not increase leakage, but will allow more performance uses for structs.

[dzmitry-lahoda]

This is code modification in our repo. I have not tested performance (and if I would I have to test not only .NET Сore, but Unity either). I assume that 200+ byte structs will work better that way, then init and as default into index and then into internal array, may be need some test so. preallocate is not good work, something better?

-        public PooledList(int capacity) : this(capacity, ClearMode.Auto, ArrayPool<T>.Shared) { }
+        public PooledList(int capacity) : this(capacity, ClearMode.Auto, ArrayPool<T>.Shared, false) { }

         /// <summary>
         /// Constructs a List with a given initial capacity. The list is
         /// initially empty, but will have room for the given number of elements
         /// before any reallocations are required.
         /// </summary>
-        public PooledList(int capacity, ClearMode clearMode) : this(capacity, clearMode, ArrayPool<T>.Shared) { }
+        public PooledList(int capacity, bool preallocate) : this(capacity, ClearMode.Auto, ArrayPool<T>.Shared, preallocate) { }

         /// <summary>
         /// Constructs a List with a given initial capacity. The list is
         /// initially empty, but will have room for the given number of elements
         /// before any reallocations are required.
         /// </summary>
-        public PooledList(int capacity, ArrayPool<T> customPool) : this(capacity, ClearMode.Auto, customPool) { }
+        public PooledList(int capacity, ClearMode clearMode) : this(capacity, clearMode, ArrayPool<T>.Shared, false) { }
+
+        public PooledList(int capacity, ClearMode clearMode, bool preallocate) : this(capacity, clearMode, ArrayPool<T>.Shared, preallocate) { }
+
+        /// <summary>
+        /// Constructs a List with a given initial capacity. The list is
+        /// initially empty, but will have room for the given number of elements
+        /// before any reallocations are required.
+        /// </summary>
+        public PooledList(int capacity, ArrayPool<T> customPool) : this(capacity, ClearMode.Auto, customPool, false) { }

         /// <summary>
         /// Constructs a List with a given initial capacity. The list is
         /// initially empty, but will have room for the given number of elements
         /// before any reallocations are required.
         /// </summary>
-        public PooledList(int capacity, ClearMode clearMode, ArrayPool<T> customPool)
+        public PooledList(int capacity, ClearMode clearMode, ArrayPool<T> customPool, bool preallocate)
         {
             if (capacity < 0)
                 ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument.capacity, ExceptionResource.ArgumentOutOfRange_NeedNonNegNum);
@@ -126,6 +135,11 @@ namespace Collections.Pooled
             {
                 _items = _pool.Rent(capacity);
             }
+
+            if (preallocate)
+            {
+                _size = capacity;
+            }

[jtmueller]

In the interest of not exploding an already-large number of constructors by adding an extra parameter, what would you think of instead adding a SetCapacity method? This would pretty much duplicate the existing Capacity property, except that you could optionally pass a preallocate boolean that would set the size equal to the desired capacity. Usage would be something like...

players = new PooledList<PlayerState>();
players.SetCapacity(Players.Count, preallocate: true);

[dzmitry-lahoda]

I dislike that will have to pass count 2 times:

players = new PooledList<PlayerState>(0);
players.SetCapacity(Players.Count, preallocate: true);

or

players = new PooledList<PlayerState>(Players.Count);
players.SetCapacity(Players.Count, preallocate: true);

Need to think.

[jtmueller]

Technically you don't have to pass the count 2 times: just don't pass anything into the constructor, because one line later you're setting the capacity. But I get that two lines of code isn't as nice as one line of code, and maybe adding more constructor overloads would be worth it.

However, there's another thing to consider when pre-allocating that your sample code doesn't account for. What if the array we get from the pool already has values in it? In the current published code this doesn't matter, because the only way to move the internal _size pointer is to overwrite whatever is currently in the array we rented.

By using preallocate to directly move the _size pointer, we're exposing the random contents of pooled arrays to users who are probably expecting a newly-created list to only have default values in it. This probably means that when preallocate == true we should clear the array we just rented. Do you see this as a performance problem?

[dzmitry-lahoda]

I guess if not to pass 0 capacity, there will be default array of size N to be rented, no need for it.

I do not need array be cleared as I do full copy over into it. Also I really use ClearMode.Never, in this case free of rented array(on dispose) and either for prealocated could not clear on creation. In other case it should clear preallocated (not sure if usage for this - but cleaner semantics). So I pass both - prellocated and ClearMode into ctor.

SetCapacity, in List there is Capacity setter which throw on count larger. So better not use that word.

Why we need list at all? Each time we copy game world into history (essentially allocated new world, copy it into new place, apply network diff). And during next wold tick world can grow, but most times not. So we still need array list semantics (and all nice api), we cannot use static length arrays.

Other option - when world moved whole cycle of approx 60 ticks (2 seconds), we reuse PooledList objects (ObjectPool), but this is more changes to our code and another pool. I.e. this is not gradual evolutionary development which could be still allowed by preallocated.

So:

1. List, slow, GC, no Span access
2. Huge perf boos and simple - PooledList, much much less GC, Span access (with ref).
3. Perf boost and simple - Preallocated PooledList
4. Not so perf boost and complex code - ObjectPool of PooledList, complex procoll of reuse and compaction via set of Capacity

[dzmitry-lahoda]

I can do next pull:

• add new ctor parameter into all ctors
• name it well (seen of better world - guess I can look into C++) and properly document
• ensure old ctor users are not broken and do not use defaults in ctor
• cover with tests
• ensure preallocate respects ClearMode (pre clear if needed)

Alternative:

• manual ctor with 0 to avoid first rent
• call SetCapacity(bad name) or SetCount or Count setter or Preallocate or SetSize method right after with respected ClearMode
• I dislike alternative, still may pull this (so not sure if it eventually and consistently better than ctor).

[dzmitry-lahoda]

https://en.cppreference.com/w/cpp/container/vector/resize https://en.cppreference.com/w/cpp/container/vector/vector

In short, C++ Vector has requested feature, but same time whole API is somewhat different.

Read not only header, but bottom either. As it seems most important here. E.g. they have ctor which is

explicit vector( size_type count );
3) Constructs the container with count default-inserted instances of T. No copies are made.

Also

5) Copy constructor. Constructs the container with the copy of the contents of other. If alloc is not provided, allocator is obtained as if by calling 

In my case I have large structs starting from 20 bytes (I guess 200 or so). So I want to directly clone by ref via Span https://en.cppreference.com/w/cpp/container/vector/data and then build up internal GC heap references, like:

    /// <summary>
        /// Clones this instance.
        /// </summary>
        public static void Clone(this in PlayerState self, ref PlayerState to)
        {
            UnsafeClone.Clone(in self, ref to);
            PlayerState.CloneReferences(in self, ref to);
        }
    internal static void CloneReferences(in PlayerState from, ref PlayerState to)
        {
            to.Shots = from.Shots != null ? new PooledList<ShotInfo>(from.Shots.Span) : null;
            ....
        }

So my case could be

explicit vector( size_type count, const Allocator& alloc = Allocator() );

where I use NonCleaningNonDefaultingAllocator... So complex topic for API.

So not sure if C++ has unsafe API which does not clean data, but may be have.

[jtmueller]

I like your initial proposal. I think it's better than the alternative. If you're willing to submit a PR for it, I would welcome that.