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

isdom · 2017-01-26T16:14:10Z

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:

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

The text was updated successfully, but these errors were encountered:

normanmaurer · 2017-01-26T17:41:05Z

@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.

isdom · 2017-01-27T03:15:50Z

@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 · 2017-01-27T07:26:36Z

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

isdom · 2017-01-27T07:35:56Z

@normanmaurer OK，Let me try...

normanmaurer · 2017-01-27T07:39:05Z

@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 👍

… for normal allocations.

Motivation: Disable ThreadLocal Cache, then allocate Pooled ByteBuf and release all these buffers, PoolArena's tiny/small/normal allocation count is incorrect. Modifications: - Calculate PoolArena's tiny/small/normal allocation one time - Add testAllocationCounter TestCase Result: Fixes #6282 .

normanmaurer · 2017-01-30T09:45:35Z

Fixed by #6288

Motivation: Disable ThreadLocal Cache, then allocate Pooled ByteBuf and release all these buffers, PoolArena's tiny/small/normal allocation count is incorrect. Modifications: - Calculate PoolArena's tiny/small/normal allocation one time - Add testAllocationCounter TestCase Result: Fixes netty#6282 .

normanmaurer closed this as completed Jan 26, 2017

normanmaurer reopened this Jan 27, 2017

isdom added a commit to isdom/netty that referenced this issue Jan 27, 2017

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

194bb24

isdom mentioned this issue Jan 27, 2017

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

Closed

isdom added a commit to isdom/netty that referenced this issue Jan 28, 2017

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

e0b79fe

… for normal allocations.

isdom added a commit to isdom/netty that referenced this issue Jan 28, 2017

fix netty#6282: erase trailing whitespace

25a3ae9

isdom added a commit to isdom/netty that referenced this issue Jan 28, 2017

fix netty#6282: erase trailing whitespace

d0d3a39

normanmaurer closed this as completed in f10f8a3 Jan 30, 2017

normanmaurer added this to the 4.0.44.Final milestone Jan 30, 2017

normanmaurer added the defect label Jan 30, 2017

normanmaurer self-assigned this Jan 30, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

isdom commented Jan 26, 2017 •

edited

normanmaurer commented Jan 26, 2017

isdom commented Jan 27, 2017 •

edited

normanmaurer commented Jan 27, 2017

isdom commented Jan 27, 2017

normanmaurer commented Jan 27, 2017

normanmaurer commented Jan 30, 2017

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

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

Comments

isdom commented Jan 26, 2017 • edited

Expected behavior

Actual behavior

Steps to reproduce

Minimal yet complete reproducer code (or URL to code)

Netty version

JVM version (e.g. java -version)

OS version (e.g. uname -a)

normanmaurer commented Jan 26, 2017

isdom commented Jan 27, 2017 • edited

normanmaurer commented Jan 27, 2017

isdom commented Jan 27, 2017

normanmaurer commented Jan 27, 2017

normanmaurer commented Jan 30, 2017

isdom commented Jan 26, 2017 •

edited

JVM version (e.g. `java -version`)

OS version (e.g. `uname -a`)

isdom commented Jan 27, 2017 •

edited