Conversation
… a persistent reader handle from buffer
… passing specific allocator for creating a new bufferHandle
ShadauxCat
left a comment
There was a problem hiding this comment.
Would like to have a more descriptive title for this PR, like "Allow passing an allocator for FastBufferReader's internal handle when using Allocator.None"... it's a little more wordy but expresses why the change is being made, which I think is more important to express than what the change is in the commit summary.
Otherwise good.
|
Thanks, @ShadauxCat! And done! Wording things is hard! |
| } | ||
|
|
||
| private static unsafe ReaderHandle* CreateHandle(byte* buffer, int length, int offset, Allocator allocator) | ||
| private static unsafe ReaderHandle* CreateHandle(byte* buffer, int length, int offset, Allocator allocator, Allocator handleAllocator) |
There was a problem hiding this comment.
so now we have Allocator allocator and adding Allocator handleAllocator. what's going on? can't someone just specify allocator = bla instead of allocator = none, handleAllocator = bla?
There was a problem hiding this comment.
When passing allocator = Allocator.None, it uses the buffer you pass in directly without any copies, but it still has to make an allocation for the metadata (position, length, etc). Before this change, that always used Allocator.Temp, but that doesn't work if you want to reference an existing buffer and keep the reader around for any length of time at all (it automatically deallocates when the stack context is left), so this change is to allow a buffer to be referenced without copying, while allocating the handle using Allocator.Persistent so it can last longer.
There was a problem hiding this comment.
gotcha, perhaps better parameter names and xmldoc then? :)
…internalAllocator and the copyAllocator is set to none, we want to make sure we are disposing the handle with the right label
The change within this PR is required to allow users to create FastBuffer Reader and Writers pools, as before the FastBufferReader's internal handle was always allocated via a Temp allocator, with this change a user can pass in a Persistent allocator.
Changelog
Testing and Documentation