-
Notifications
You must be signed in to change notification settings - Fork 580
Implementing ScratchBufferAllocator #1247
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 implements the new ScratchAllocationManager to manage memory allocations without copying existing data and updates all relevant usages from ScratchBufferManager to ScratchAllocationManager.
- Implement new ScratchAllocationManager with backup buffer reuse
- Update transaction, response, and custom command code to use ScratchAllocationManager
- Adjust unit tests to validate behavior of the new manager
Reviewed Changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
test/Garnet.test/ScratchAllocationManagerTests.cs | Added tests to verify correct allocation, rewind, and reset behavior |
libs/server/Transaction/TxnRespCommands.cs | Updated custom procedure invocation to use ScratchAllocationManager |
libs/server/Transaction/TransactionManager.cs | Replaced scratch buffer manager with scratch allocation manager |
libs/server/Resp/RespServerSession.cs | Initialized and reset the new ScratchAllocationManager alongside the old manager |
libs/server/Custom/CustomTransactionProcedure.cs | Updated method calls to use ScratchAllocationManager |
libs/server/Custom/CustomRespCommands.cs | Updated call to GetCustomTransactionProcedure to use the allocation manager |
libs/server/Custom/CustomCommandManagerSession.cs | Revised method signature and assignment for scratch allocation manager |
libs/server/ArgSlice/ScratchAllocationManager.cs | New implementation of ScratchAllocationManager with expanded buffer logic |
libs/common/RefStack.cs | Updated to use new C# 12 array initialization for an empty stack |
ScratchBufferAllocator
is similar toScratchBufferBuilder
(previouslyScratchBufferManager
), but instead of creating a new buffer and copying the previous data when the buffer has no sufficient space (and releasing the pointer to the previous buffer), it instead keeps a stack of the previous buffers and does not copy the previous data.By doing so,
ScratchBufferAllocator
ensures that any previously createdArgSlice
s that have not been explicitly re-winded do not point to GCed memory.The reason that both managers are currently necessary, is that
ScratchBufferBuilder
supports getting all the data in a continuous chunk of memory, whereScratchBufferAllocator
does not.