-
Notifications
You must be signed in to change notification settings - Fork 583
Avoid memory allocation for hash object operations wherever possible #1248
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR optimizes hash object operations by conditionally using ReadOnlySpan-based implementations (for NET9_0_OR_GREATER) to avoid unnecessary memory allocations and reuse existing memory buffers where possible. Key changes include switching from ToByteArray() to AsReadOnlySpan() for key extraction, conditionally copying values only when their sizes differ, and updating method signatures to use a unified ByteSpan type.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
libs/server/Objects/Hash/HashObjectImpl.cs | Updated key/value extraction and memory reuse logic with conditional compilation for NET9_0_OR_GREATER. |
libs/server/Objects/Hash/HashObject.cs | Modified API methods to use the new ByteSpan type and alternate lookup mechanisms. |
libs/common/RespMemoryWriter.cs | Changed WriteIntegerFromBytes to accept a ReadOnlySpan instead of a byte array. |
Didn't think of using type alias for the key argument, makes this much cleaner than my previous attempt 👍 |
@PaulusParssinen After the PR merge, maybe you can also commit your multi lookup optimization? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Functionally looks good to me, perhaps needs some slight tweaks to make code more readable. LMK what you think.
Currently, most operations on Hash Objects allocate a
key
value to lookup the stored value. The aim here is to:key
instance for expireTime (>= net.9)value
array when replacing a value of the same size