StackExchange / StackExchange.Redis

General purpose redis client
https://stackexchange.github.io/StackExchange.Redis/
Other
5.91k stars 1.51k forks source link

Passing a pooled array to HashSet/StreamAdd and similar methods. #2027

Open NimaAra opened 2 years ago

NimaAra commented 2 years ago

I have a usecase where allocations need to be minimized and there are scenarios where I get a T[] as an input that needs to be converted to either NameValueEntry[] or HashEntry[] before I can invoke StreamAdd() or HashSet().

One way to tackle this would be to create a buffer using the ArrayPool e.g.

T[] incoming = ...
NameValueEntry[] buffer = ArrayPool<NameValueEntry>.Shared.Rent(incoming.Length)
// copy the items from incoming to buffer
db.StreamAdd(key, buffer);

However given that the buffer.Length returned by the pool is not necessarily same as the requested incoming.Length there is no way for me to indicate how many items StreamAdd()/HashSet() should take.

Would it be possible to create an overload which could support this?

mgravell commented 2 years ago

Yes, this is indeed a limitation currently. We should probably add an API that allows a ReadOnlyMemory<RedisValue> (etc) instead of just a RedisValue[] (for both the sync and async APIs; we need to add the data to an internal object to pass through the call-path, which makes ReadOnlySpan<T> awkward to use), but we'd need to think about which APIs. Due to the volume involved, I would not propose adding this wildly.

Let's see if we can start a list of likely candidates; perhaps

strong case:

maybe?

meh...?

others?

NimaAra commented 2 years ago

ReadOnlyMemory<T> sounds like the best approach.

Can something similar be done for the return effectively allowing the user to either pass in a buffer for the output of methods such as StreamRead/HashGetAllAsync/HashGetAsync which return an array or something similar to a lease which the user would use and return once done... This would eliminate allocations in the opposite direction.

mgravell commented 2 years ago

See StringGetLease - that's similar but related, has overlapping API surface

NimaAra commented 2 years ago

Yes been using StringGetLease for a while now, would be good to expand it to others, I have many usecases some with largish arrays which would benefit from this...

NimaAra commented 2 years ago

Is this something that I can contribute to? (assuming you are focusing on other stuff)

Stabzs commented 2 years ago

@mgravell I put together a draft PR for the StringSet/StringSetAsync operation to see if the implementation is on the right track. I'm happy to continue expanding the branch if it is the approach you were looking for. https://github.com/StackExchange/StackExchange.Redis/pull/2221