Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix #6282: calculate correct count for PoolArena's tiny/small/normal allocation #6288

Closed
wants to merge 4 commits into from
Closed

fix #6282: calculate correct count for PoolArena's tiny/small/normal allocation #6288

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
194bb24
fix #6282: calculate correct count for tiny/small/normal allocation
isdom Jan 27, 2017
e0b79fe
fix #6282: acquire the synchronized(this) once when we increment for
isdom Jan 28, 2017
25a3ae9
fix #6282: erase trailing whitespace
isdom Jan 28, 2017
d0d3a39
fix #6282: erase trailing whitespace
isdom Jan 28, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Prev Previous commit
Next Next commit
fix #6282: acquire the synchronized(this) once when we increment for 
normal allocations.
  • Loading branch information
@isdom
isdom committed Jan 28, 2017
commit e0b79fe7e7e70ccaf5c3178be773c0a8a552a1fe
45 changes: 23 additions & 22 deletions buffer/src/main/java/io/netty/buffer/PoolArena.java
View file
Open in desktop
Expand Up @@ -190,41 +190,44 @@ private void allocate(PoolThreadCache cache, PooledByteBuf<T> buf, final int req

final PoolSubpage<T> head = table[tableIdx];

try {
/**
* Synchronize on the head. This is needed as {@link PoolChunk#allocateSubpage(int)} and
* {@link PoolChunk#free(long)} may modify the doubly linked list as well.
*/
synchronized (head) {
final PoolSubpage<T> s = head.next;
if (s != head) {
assert s.doNotDestroy && s.elemSize == normCapacity;
long handle = s.allocate();
assert handle >= 0;
s.chunk.initBufWithSubpage(buf, handle, reqCapacity);
return;
}
/**
* Synchronize on the head. This is needed as {@link PoolChunk#allocateSubpage(int)} and
* {@link PoolChunk#free(long)} may modify the doubly linked list as well.
*/
synchronized (head) {
final PoolSubpage<T> s = head.next;
if (s != head) {
assert s.doNotDestroy && s.elemSize == normCapacity;
long handle = s.allocate();
assert handle >= 0;
s.chunk.initBufWithSubpage(buf, handle, reqCapacity);
incTinySmallNormalAllocation(normCapacity);
return;
}
}
synchronized (this) {
allocateNormal(buf, reqCapacity, normCapacity);
return;
} finally {
incTinySmallNormalAllocation(normCapacity);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@isdom I would prefer if we not need to acquire the synchronized(this) two times when we increment for normal allocations. Can you please refactor to ensure we not do ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@normanmaurer Could you give me some advice for my commit: acquire the synchronized(this) once when we increment for normal allocations?

}
return;

}
if (normCapacity <= chunkSize) {
if (cache.allocateNormal(this, buf, reqCapacity, normCapacity)) {
// was able to allocate out of the cache so move on
return;
}
allocateNormal(buf, reqCapacity, normCapacity);
incTinySmallNormalAllocation(normCapacity);
synchronized (this) {
allocateNormal(buf, reqCapacity, normCapacity);
incTinySmallNormalAllocation(normCapacity);
}
} else {
// Huge allocations are never served via the cache so just call allocateHuge
allocateHuge(buf, reqCapacity);
}
}

private synchronized void allocateNormal(PooledByteBuf<T> buf, int reqCapacity, int normCapacity) {
private void allocateNormal(PooledByteBuf<T> buf, int reqCapacity, int normCapacity) {
if (q050.allocate(buf, reqCapacity, normCapacity) || q025.allocate(buf, reqCapacity, normCapacity) ||
q000.allocate(buf, reqCapacity, normCapacity) || qInit.allocate(buf, reqCapacity, normCapacity) ||
q075.allocate(buf, reqCapacity, normCapacity)) {
Expand All @@ -245,9 +248,7 @@ private void incTinySmallNormalAllocation(final int normCapacity) {
} else if (isTinyOrSmall(normCapacity)) {
allocationsSmall.increment();
} else {
synchronized (this) {
++allocationsNormal;
}
++allocationsNormal;
}
}

Expand Down