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.

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

NormalMemoryRegionCache's arraysize is over booked #2925

Closed
garretwu opened this issue Sep 22, 2014 · 10 comments
Closed

NormalMemoryRegionCache's arraysize is over booked #2925

garretwu opened this issue Sep 22, 2014 · 10 comments
Assignees
Labels
Milestone

Comments

@garretwu
Copy link
Contributor

When create NormalMemoryRegionCache for PoolThreadCache. The array size is decided by:
int max = Math.min(area.chunkSize, maxCachedBufferCapacity);
int arraySize = Math.max(1, max / area.pageSize);

So most likely it is decided by maxCachedBufferCapacity/area.pageSize, with maxCachedBufferCapacity=2M the array size will be 2^8 = 256.

when we try to get normal cache the index is pre-processed by log2():
PoolThreadCache.cacheForNormal():
int idx = log2(normCapacity >> numShiftsNormalHeap);
return cache(normalHeapCaches, idx);

A 2M cache entry will get from the 8th cache bin(start from 0).

The space for NormalMemoryRegionCache is over booked, though it will not impact correctness(just a little performance..).

@normanmaurer normanmaurer added this to the 4.0.24.Final milestone Sep 22, 2014
@normanmaurer
Copy link
Member

@garretwu thanks for reporting... I will check and fix it

@normanmaurer
Copy link
Member

@garretwu I'm right you say it should get the 7th cache bin(start from0) . right ?

@normanmaurer normanmaurer self-assigned this Oct 30, 2014
@garretwu
Copy link
Contributor Author

you are right, @normanmaurer. That is what I mean.

@normanmaurer
Copy link
Member

Thanks for clarify!

Am 12.11.2014 um 03:52 schrieb garretwu notifications@github.com:

you are right, @normanmaurer. That is what I mean.


Reply to this email directly or view it on GitHub.

@normanmaurer
Copy link
Member

Totally forgot about this. Will look into it or would you want to submit a pr @garretwu ? We love contributions :)

@normanmaurer
Copy link
Member

@garretwu what you think about provide a PR for this ? I'm still not 100 % sure I understand the issue correctly, so maybe it would be the easiest to just have you fix it ;)

@garretwu
Copy link
Contributor Author

sure, I could create a PR:)

@normanmaurer
Copy link
Member

This would be awesome!

Am 11.04.2015 um 00:02 schrieb garretwu notifications@github.com:

sure, I could create a PR:)


Reply to this email directly or view it on GitHub.

@garretwu
Copy link
Contributor Author

Hi @normanmaurer.
PR: #3624 is created for such issue.

mvn test pass.

end to end test:
Dumped heap to check.
with Dio.netty.allocator.maxCachedBufferCapacity: 32768 (cache max 32K),
NormalMemoryRegionCache size is 3(8k, 16k, 32k), rather than previous 4.

normanmaurer pushed a commit that referenced this issue Apr 13, 2015
…Cache

Motivation:

When create NormalMemoryRegionCache for PoolThreadCache, we overbooked
cache array size. This means unnecessary overhead for thread local cache
as we will create multi cache enties for each element in cache array.

Modifications:

change:
int arraySize = Math.max(1, max / area.pageSize);
to:
int arraySize = Math.max(1, log2(max / area.pageSize) + 1);

Result:

Now arraySize won't introduce unnecessary overhead.

 Changes to be committed:
	modified:   buffer/src/main/java/io/netty/buffer/PoolThreadCache.java
normanmaurer pushed a commit that referenced this issue Apr 13, 2015
…Cache

Motivation:

When create NormalMemoryRegionCache for PoolThreadCache, we overbooked
cache array size. This means unnecessary overhead for thread local cache
as we will create multi cache enties for each element in cache array.

Modifications:

change:
int arraySize = Math.max(1, max / area.pageSize);
to:
int arraySize = Math.max(1, log2(max / area.pageSize) + 1);

Result:

Now arraySize won't introduce unnecessary overhead.

 Changes to be committed:
	modified:   buffer/src/main/java/io/netty/buffer/PoolThreadCache.java
normanmaurer pushed a commit that referenced this issue Apr 13, 2015
…Cache

Motivation:

When create NormalMemoryRegionCache for PoolThreadCache, we overbooked
cache array size. This means unnecessary overhead for thread local cache
as we will create multi cache enties for each element in cache array.

Modifications:

change:
int arraySize = Math.max(1, max / area.pageSize);
to:
int arraySize = Math.max(1, log2(max / area.pageSize) + 1);

Result:

Now arraySize won't introduce unnecessary overhead.

 Changes to be committed:
	modified:   buffer/src/main/java/io/netty/buffer/PoolThreadCache.java
@normanmaurer
Copy link
Member

Fixed by #3624

pulllock pushed a commit to pulllock/netty that referenced this issue Oct 19, 2023
…hreadCache

Motivation:

When create NormalMemoryRegionCache for PoolThreadCache, we overbooked
cache array size. This means unnecessary overhead for thread local cache
as we will create multi cache enties for each element in cache array.

Modifications:

change:
int arraySize = Math.max(1, max / area.pageSize);
to:
int arraySize = Math.max(1, log2(max / area.pageSize) + 1);

Result:

Now arraySize won't introduce unnecessary overhead.

 Changes to be committed:
	modified:   buffer/src/main/java/io/netty/buffer/PoolThreadCache.java
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants