Improved pooling of buffers when a buffer was released in other thread. #73
Conversation
|
Hi @hide1202, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution! TTYL, AZPRBOT; |
hide1202
force-pushed
the
feature/improve-pool
branch
from
11b6162 to
5296408
Compare
| @@ -41,21 +50,307 @@ public bool Release<T>(T value) | |||
| } | |||
| } | |||
|
|
|||
| internal sealed class Stack : Stack<Handle> | |||
| internal sealed class WeakOrderQueue | |||
There was a problem hiding this comment.
@nayato If WeakOrderQueue were private, Stack couldn't access WeakOrderQueue. So WeakOrderQueue class has internal modifier.
There was a problem hiding this comment.
I see, differences in Java vs .NET scope mechanics, ok.
hide1202
force-pushed
the
feature/improve-pool
branch
from
fae24db to
2613b0d
Compare
| class Link | ||
| { | ||
| internal int readIndex; | ||
| internal volatile int writeIndex; |
There was a problem hiding this comment.
pls switch to Volatile.Read/Write methods. see http://blog.alexrp.com/2014/03/30/dot-net-atomics-and-memory-model-semantics/ for details
|
pls check the comments above. I'll do another pass tomorrow with a fresh head. Thanks for taking on this. That's actually a great addition to an honest port of pooled byte buffer allocator I'm working on. |
|
@nayato OK, I will check your comments. And i check more detailedly and i will fix it. And i will add commit in order that you check corrections easily. If you would check, i will rebase it. 😄 |
hide1202
force-pushed
the
feature/improve-pool
branch
2 times, most recently
from
d6fbcbd to
7fd8994
Compare
|
Can you also port the tests from https://github.com/netty/netty/blob/4.1/common/src/test/java/io/netty/util/RecyclerTest.java? |
|
@nayato OK, i will check your comments. |
|
btw, with this change the reason for renaming Recycler into ThreadLocalPool is not there anymore so it would make sense to rename it back into Recycler. Not necessarily in this PR if it's a big change. |
| writeIndex = Volatile.Read(ref tail.writeIndex); | ||
| } | ||
| tail.elements[writeIndex] = handle; | ||
| //handle.Stack = null; |
There was a problem hiding this comment.
why is it commented out? If it's because Handle.Stack is readonly - remove the readonly.
|
@nayato I move ThreadLocalPoolTest to DotNetty.Common.Tests. (for accessing internal method) |
hide1202
force-pushed
the
feature/improve-pool
branch
2 times, most recently
from
7dfa906 to
7541be9
Compare
| where T : class | ||
| { | ||
| Contract.Requires(value == this.Value, "value differs from one backed by this handle."); | ||
|
|
||
| Stack stack = this.Stack; | ||
| if (stack.Thread != Thread.CurrentThread) | ||
| if (stack.Thread == Thread.CurrentThread) |
There was a problem hiding this comment.
used twice => cache in local variable
| internal Handle[] elements; | ||
|
|
||
| private int maxCapacity; | ||
| internal int head; |
|
@hide1202 now I think I'm done reviewing and it's OK to rebase/squash once this last batch of comments is out of the way. |
hide1202
force-pushed
the
feature/improve-pool
branch
from
7541be9 to
d43e6cf
Compare
Motivation: When buffer was released in other thread, this buffer couldn't be released. I think that need to release a buffer in other thread for memory effectiveness. Modifications: Implement WeakOrderQueue for releasing a buffer that other thread owned. Result: When buffer was released in othread thread, owned thread can use this buffer.
hide1202
force-pushed
the
feature/improve-pool
branch
from
d43e6cf to
5b1139e
Compare
|
@nayato I checked your comments, and fixed all. And rebase to one commit. 😃 |
Improved pooling of buffers when a buffer was released in other thread.
|
@hide1202 thanks for taking on this! |
resolve #63
Improved pooling of buffers when a buffer was released in other thread.
Motivation:
When buffer was released in other thread, this buffer couldn't be released. I think that need to release a buffer in other thread for memory effectiveness.
Modifications:
Implement WeakOrderQueue for releasing a buffer that other thread owned.
Result:
When buffer was released in othread thread, owned thread can use this buffer.