Skip to content

Incorrect allocations value for PoolArena (tiny / small / normal) #6282

New issue
New issue
Closed
Closed
Incorrect allocations value for PoolArena (tiny / small / normal)#6282
Assignees
normanmaurer
Labels
defect
Milestone
 
4.0.44.Final
@isdom

Description

@isdom
isdom
opened on Jan 26, 2017 · edited by isdom
Contributor

I found some PoolArena allocations value was incorrect, more specifically is:

long numTinyAllocations();
long numSmallAllocations();
long numNormalAllocations();

and

long numNormalDeallocations();

When I alloc Pooled ByteBuf & and release all these Bufs,

Expected behavior

PoolArena's Metric SHOULD meet the following conditions:

           numTinyAllocations() == numTinyDeallocations()
     &&   numSmallAllocations() == numSmallDeallocations()
     &&   numNormalAllocations() == numNormalDeallocations()

Actual behavior

BUT now result:

           numTinyAllocations() < numTinyDeallocations()
     &&   numSmallAllocations() < numSmallDeallocations()
     &&   numNormalAllocations() > numNormalDeallocations()

It seems some tiny & small allocations increase normal's counter, I export PoolArenaMetric as MBean by code and show MBean by Web using zkoss, see below:

2017-01-25 11 23 34

In this case,

numAllocations(2404) == numDeallocations(2404)

BUT

   (numTinyDeallocations - numTinyAllocations)       //  == 17
+ (numSmallDeallocations - numSmallAllocations)       //  == 9

equals

(numNormalAllocations - numNormalDeallocations) // == 26

Steps to reproduce

I start test code with VM flag (disable thread local cache):

-XX:MaxDirectMemorySize=96M
-Dio.netty.allocator.tinyCacheSize=0 
-Dio.netty.allocator.smallCacheSize=0 
-Dio.netty.allocator.normalCacheSize=0 
-Dio.netty.allocator.type=pooled

then alloc some ByteBuf and release Bufs.

Minimal yet complete reproducer code (or URL to code)

TestCase to reproduce: https://github.com/isdom/jocean-http/blob/6bc6cfa9ae1e71395a4dd52355b1b0a8365bb530/jocean-http/src/test/java/org/jocean/netty/buffer/PooledByteBufAllocatorTestCase.java

and make sure run this test with VM flag: -XX:MaxDirectMemorySize=96M

It fail at:

assertEquals(metric.numTinyDeallocations(), metric.numTinyAllocations());
assertEquals(metric.numSmallDeallocations(), metric.numSmallAllocations());
assertEquals(metric.numNormalDeallocations(), metric.numNormalAllocations());

output :

java.lang.AssertionError: expected:<1> but was:<0>
	at org.junit.Assert.fail(Assert.java:88)

If this is confirmed as a issue，I can open a PR to fix it.

Netty version

netty-all-4.1.7.Final

JVM version (e.g. java -version)

java version "1.8.0_102"
Java(TM) SE Runtime Environment (build 1.8.0_102-b14)
Java HotSpot(TM) 64-Bit Server VM (build 25.102-b14, mixed mode)

OS version (e.g. uname -a)

Linux xxx 3.10.0-327.22.2.el7.x86_64 #1 SMP Thu Jun 23 17:05:11 UTC 2016 x86_64 x86_64 x86_64 GNU/Linux

Activity

normanmaurer

normanmaurer commented on Jan 26, 2017

@normanmaurer
normanmaurer
on Jan 26, 2017
Member

@isdom this is because of the caches... when you deallocate the buffers they will first end up in ThreadLocal caches. Only after these removed from the caches they are really deallocated. You can disable the caches to see it.

normanmaurer
closed this as completedon Jan 26, 2017
isdom

isdom commented on Jan 27, 2017

@isdom
isdom
on Jan 27, 2017 · edited by isdom
ContributorAuthor

@normanmaurer but I have already disable ThreadLocal caches by set JVM flag:

-Dio.netty.allocator.tinyCacheSize=0 
-Dio.netty.allocator.smallCacheSize=0 
-Dio.netty.allocator.normalCacheSize=0

and Take a look at the following test code:

    public final void testPoolArenaAllocationCounter() {
        // create PooledByteBufAllocator instance with ThreadLocal Cache disabled
        final PooledByteBufAllocator allocator = new PooledByteBufAllocator(
                true,   //boolean preferDirect, 
                0,      //int nHeapArena, 
                1,      //int nDirectArena, 
                8192,   //int pageSize, 
                11,     //int maxOrder,
                0,      //int tinyCacheSize, 
                0,      //int smallCacheSize, 
                0,      //int normalCacheSize,
                true    //boolean useCacheForAllThreads
                );
        
        // alloc tiny buf
        final ByteBuf b1 = allocator.directBuffer(24);
        
        // alloc small buf
        final ByteBuf b2 = allocator.directBuffer(800);
        
        // alloc normal buf
        final ByteBuf b3 = allocator.directBuffer(8192 * 2);
        
       // make sure alloc buf success 
        assertNotNull(b1);
        assertNotNull(b2);
        assertNotNull(b3);
        
        // then release buf to deallocated buffers memory, and bcs threadlocal cache has been disabled
        // allocations counter value must equals deallocations counter value
        assertTrue(b1.release());
        assertTrue(b2.release());
        assertTrue(b3.release());
        
        assertTrue(allocator.directArenas().size() >= 1);
        
        final PoolArenaMetric metric = allocator.directArenas().get(0);
        
        // 1 tiny allocation + 1 small allocation + 1 normal allocation
        assertEquals(3, metric.numDeallocations());    // assertion PASS
        assertEquals(3, metric.numAllocations());        // assertion PASS
        
        // BUT following assertion fail
        assertEquals(metric.numTinyDeallocations(), metric.numTinyAllocations());
        assertEquals(metric.numSmallDeallocations(), metric.numSmallAllocations());
        assertEquals(metric.numNormalDeallocations(), metric.numNormalAllocations());
    }

I think PoolArena try to allocate tiny or small buffer, when PoolSubpage is empty( head.next == head ), tiny or small buffer initialized via function allocateNormal(buf, reqCapacity, normCapacity) , but allocateNormal always increase normal's counter at code1 and code2, these two increment action could be replaced by following code:

private void updateAllocationCounter(final int normCapacity) {
        if (isTiny(normCapacity)) {
            allocationsTiny.increment();
        } else if (isTinyOrSmall(normCapacity)) {
            allocationsSmall.increment();
        } else {
            ++allocationsNormal;
        }
}

Please check PoolArena's allocation increment code (tiny / small / normal ) again, thanks @normanmaurer

normanmaurer

normanmaurer commented on Jan 27, 2017

@normanmaurer
normanmaurer
on Jan 27, 2017
Member

@isdom doh! You are right. Want to do a PR or should I take care ?

normanmaurer
reopened this on Jan 27, 2017
isdom

isdom commented on Jan 27, 2017

@isdom
isdom
on Jan 27, 2017
ContributorAuthor

@normanmaurer OK，Let me try...

normanmaurer

normanmaurer commented on Jan 27, 2017

@normanmaurer
normanmaurer
on Jan 27, 2017
Member

@isdom ok cool... Would be nice if just would calculate if its tiny/small/normal allocation one time though and not need to do multiple times 👍

isdom
added a commit that references this issue on Jan 27, 2017

fix netty#6282: calculate correct count for tiny/small/normal allocation

194bb24
isdom
mentioned this on Jan 27, 2017
isdom
added 3 commits that reference this issue on Jan 28, 2017

fix netty#6282: acquire the synchronized(this) once when we increment…

e0b79fe

fix netty#6282: erase trailing whitespace

25a3ae9

fix netty#6282: erase trailing whitespace

d0d3a39
normanmaurer
closed this as completedin f10f8a3on Jan 30, 2017
normanmaurer
added a commit that references this issue on Jan 30, 2017

Calculate correct count for tiny/small/normal allocation

025e656
normanmaurer

normanmaurer commented on Jan 30, 2017

@normanmaurer
normanmaurer
on Jan 30, 2017
Member

Fixed by #6288

normanmaurer
added this to the 4.0.44.Final milestone on Jan 30, 2017
normanmaurer
added
defect
on Jan 30, 2017
normanmaurer
self-assigned this
on Jan 30, 2017
liuzhengyang
added a commit that references this issue on Sep 10, 2017

Calculate correct count for tiny/small/normal allocation

ec20817
pulllock
added a commit that references this issue on Oct 19, 2023

Calculate correct count for tiny/small/normal allocation

51a9de6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

defect

Type

No type

Projects

No projects

Milestone

Relationships

None yet

    Development

    No branches or pull requests

      Participants

      @normanmaurer@isdom

      Issue actions
        Incorrect allocations value for PoolArena (tiny / small / normal) · Issue #6282 · netty/netty