[C#] Enable pooling of messages with large RepeatedFields

[ThorstenReichert]

What language does this apply to?
C# proto3

Describe the problem you are trying to solve.
Sending multiple messages, each containing a large RepeatedField (all of the same size), without allocating a new backing array for each RepeatedField. Effectively, I'd like to be able to pool RepeatedFields for reuse.

Describe the solution you'd like
One possible solution could be to provide a method similar to Clear, which keeps the backing array and only resets the count field (see here).

[jskeet]

Reassigning this as I'm not on the protobuf team - but this sort of pooling sounds pretty complicated to achieve.

[haberman]

I agree with Jon, this pooling sounds complicated. We've generally gotten good results from using arenas -- if anything we'd probably be more inclined to go in that direction.

[CodeBlanch]

Hey @haberman would you consider re-opening this and possibly accepting a PR? I don't think the ask was for protobuf to implement pooling. It was to do something like this...

        public void Clear()
          => Clear(emptyArray: true);

        public void Clear(bool emptyArray = true)
        {
            array = emptyArray ? EmptyArray : array;
            count = 0;
        }

This would allow users to implement their own pooling of RepeatedField<T>s, if desired.

For comparison, runtime List<T> works more like this in that it doesn't throw away the underlying storage when cleared: https://github.com/dotnet/runtime/blob/7704f8b5ce3fc1d5b11da7f39253f33b4f8925d6/src/libraries/System.Private.CoreLib/src/System/Collections/Generic/List.cs#L289-L305

[jskeet]

@CodeBlanch: I don't think that's actually what was being requested.

The first comment explicitly says:

Effectively, I'd like to be able to pool RepeatedFields for reuse.

Each message has a separate RepeatedField object, and changing that would be a very major change - so changing the implementation of Clear wouldn't help that scenario at all.

If the OP was talking about reusing the original message by clearing and re-populating it, that would be different (and certainly more feasible) - but that's not what's been asked for at the moment.

[ThorstenReichert]

@CodeBlanch @jskeet apologies for the unclear initial issue description. The intention of the issue was pretty much exactly what @CodeBlanch interpreted.

Instead of

Effectively, I'd like to be able to pool RepeatedFields for reuse.

Should have written something like

Effectively, I'd like to be able to reuse messages containing RepeatedFields

By now the performance of my use case is good enough for me (through other means), so I personally don't have any need for this change. But @CodeBlanch feel free to hijack this issue :-)

[jskeet]

Okay, so the proposed change would just be to the Clear method, which would keep the existing array, reset it (e.g. with Array.Clear) and set the count field to 0? That sounds more reasonable, at least. I don't promise to get to this any time soon, but it's good to make sure we're all on the same page.

[haberman]

FWIW, there is prior art for this kind of optimization in C++ protobuf. Clear() on C++ protos has traditionally held on to the sub-message objects for re-use.

[CodeBlanch]

@jskeet

Okay, so the proposed change would just be to the Clear method, which would keep the existing array, reset it (e.g. with Array.Clear) and set the count field to 0? That sounds more reasonable, at least. I don't promise to get to this any time soon, but it's good to make sure we're all on the same page.

That would work for me 👍

Happy to submit a PR if it would be helpful.

[jskeet]

@CodeBlanch: Nope, it's fine - I'll try to get round to it soon. It won't be hard, it's just a matter of finding time.