Buffer allocation strategiesBad usage patterns
In my previous post, I discussed a potential implementation for a buffer pool, and commented that it would cause issues if we had particular usage patterns.
As a reminder, here is the code:
[ThreadStatic] private static Stack<byte[]>[] _buffersBySize; private static byte[] GetBuffer(int requestedSize) { if(_buffersBySize == null) _buffersBySize = new Stack<byte[]>[32]; var actualSize = PowerOfTwo(requestedSize); var pos = MostSignificantBit(actualSize); if(_buffersBySize[pos] == null) _buffersBySize[pos] = new Stack<byte[]>(); if(_buffersBySize[pos].Count == 0) return new byte[actualSize]; return _buffersBySize[pos].Pop(); } private static void ReturnBuffer(byte[] buffer) { var actualSize = PowerOfTwo(buffer.Length); if(actualSize != buffer.Length) return; // can't put a buffer of strange size here (probably an error) if(_buffersBySize == null) _buffersBySize = new Stack<byte[]>[32]; var pos = MostSignificantBit(actualSize); if(_buffersBySize[pos] == null) _buffersBySize[pos] = new Stack<byte[]>(); _buffersBySize[pos].Push(buffer); }
Now, consider a user who uses this buffer pool, but for some reason decided to move most of the disposal code to a dedicated thread. This can happen when using large files, where disposing of the file stream can take a very long time ( flushing OS buffers, etc). I have seen several places where people had a dedicated disposal threads.
What would happen in such a scenario?
Well, we would allocate buffers in threads #1 – #10, and only return them to the buffer pool on thread #12. That would mean that we would keep allocating new buffers, but all the buffered that were returned to the pool would actually go and sit in the threads that are just releasing them. Welcome memory leak .
More posts in "Buffer allocation strategies" series:
- (09 Sep 2015) Bad usage patterns
- (08 Sep 2015) Explaining the solution
- (07 Sep 2015) A possible solution
Comments
OMG, I did not see that! Great post.
What I did see, though, is that the following should be an assertion because it seems to hide bugs:
I don't understand. That problem doesn't seem inherent to your design because it defeats the purpose. You just tell people to return the buffers as soon as they're done using them and if they can't do that then they should not use this implementation.
That would also be problematic if you had a pipeline based architecture with a producer agent producing buffers and a consumer agent processing them. I really don't like ThreadStatic fields in modern software as it really don't fit in asynchronous data flows
So why do you maintain separate pool for every thread if the buffers are allowed to be used or released in other threads? BTW what problem exactly is this pool solving?
Eli, What happens if they allocate the buffer on one thread, then release it on another?
Rafal, If you have a pool of buffers that are doing roughly the same work, you get no contention for most allocations, and roughly the same work, then you can use it for this. This is scratch code, and it was interesting enough to talk about in the blog
I like this example. It makes you think. Think of the thread static stack as a shelf in a bookcase. You create a book and put it onto one shelf. Remove it and then put it back on another shelf. Pretty soon the librarian is super upset because all the books are in all the wrong places and chaos ensues.
there is no handling that ensures duplicates on the stack. returned buffers are just pushed onto the - thread - matching stack. there is no verification that ensures that the current or any other stack is currently holding already a ref to the buffer. as a result the same buffer may be used by multiple callers the same time (as the pool returns it to different callers).
wrapping the buffer in a custom class with internal state (bound to a buffer pool) would be an easy fix that avoids locking on a - possibliy - high frequent items. as the "isBound" state is internal in such a class, there is no reference from pool to the buffer that supresses garbage collection of the item.
finally, wouldn't it be nice to use WeakReferences on the stack instead of the items itself ? currently the buffer can only grow (and has no limit at all) - when memory is under pressure, WeakReferences would allow such cleanup
ideally a hybrid solution would be nice - combination of static allocations (with hard limits on memory) and weak references for additional allocations that are - in case of requested buffers - nice to have but may gracefully abadoned if system demands memory reduction.
Comment preview