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

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

Open
NimaAra opened this issue Mar 8, 2022 · 6 comments
Open

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

NimaAra opened this issue Mar 8, 2022 · 6 comments

Comments

@NimaAra
Copy link

NimaAra commented Mar 8, 2022

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
Copy link
Collaborator

mgravell commented Mar 8, 2022

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:

  • StreamAdd(RedisKey, NameValueEntry[] streamPairs, ...)
  • StringSet(KeyValuePair<RedisKey, RedisValue>[], ...)
  • HashSet(RedisKey, HashEntry[], ...)
  • HashDelete(RedisKey, RedisValue[], ...)
  • HashGet(RedisKey, RedisValue[], ...)
  • KeyDelete(RedisKey[], ...)
  • KeyExists(RedisKey[], ...)
  • ScriptEvaluate(string, RedisKey[], RedisValue[], ...)
  • Execute(string, object[])

maybe?

  • ListLeftPush(RedisKey, RedisValue[], ...)
  • ListRightPush(RedisKey, RedisValue[], ...)

meh...?

  • Geo*
  • HyperLog*

others?

@NimaAra
Copy link
Author

NimaAra commented Mar 8, 2022

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
Copy link
Collaborator

mgravell commented Mar 8, 2022

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

@NimaAra
Copy link
Author

NimaAra commented Mar 8, 2022

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
Copy link
Author

NimaAra commented Jul 4, 2022

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

@Stabzs
Copy link

Stabzs commented Aug 18, 2022

@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. #2221

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants