Oh, sorry. Sure I will. The patch has been created before, and on my local sources I'm a bit further, I just did a cherry pick and commited those one. I will refactor and reformat this part. Sorry for that.
2012/1/5 Maurizio Cucchiara <[email protected]>: > I'm sorry but I have to ask you to uniform your code with the code style > convention. > > I know that your patch has probably been created before we reach the > consensus. > > Sent from my mobile device, so please excuse typos and brevity. > > Maurizio Cucchiara > > Il giorno 05/gen/2012 22.46, <[email protected]> ha scritto: > >> Author: bperroud >> Date: Thu Jan 5 21:46:00 2012 >> New Revision: 1227854 >> >> URL: http://svn.apache.org/viewvc?rev=1227854&view=rev >> Log: >> DIRECTMEMORY-55 : OffHeapMemoryBuffer leaks 1 byte at every allocation, >> firstmatch to check correct pointer size, return null when allocating on >> multiple buffers, report correct used size >> >> Added: >> >> incubator/directmemory/trunk/directmemory-cache/src/test/java/org/apache/directmemory/memory/test/MemoryManagerServiceImplTest.java >> Modified: >> >> incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/OffHeapMemoryBuffer.java >> >> incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/Pointer.java >> >> Modified: >> incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/OffHeapMemoryBuffer.java >> URL: >> http://svn.apache.org/viewvc/incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/OffHeapMemoryBuffer.java?rev=1227854&r1=1227853&r2=1227854&view=diff >> >> ============================================================================== >> --- >> incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/OffHeapMemoryBuffer.java >> (original) >> +++ >> incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/OffHeapMemoryBuffer.java >> Thu Jan 5 21:46:00 2012 >> @@ -99,9 +99,9 @@ public class OffHeapMemoryBuffer >> Pointer fresh = new Pointer(); >> fresh.bufferNumber = existing.bufferNumber; >> fresh.start = existing.start; >> - fresh.end = fresh.start + capacity; >> + fresh.end = fresh.start + capacity - 1; // 0 indexed >> fresh.free = true; >> - existing.start += capacity + 1; >> + existing.start = fresh.end + 1; // more readable >> return fresh; >> } >> >> @@ -110,7 +110,7 @@ public class OffHeapMemoryBuffer >> { >> for ( Pointer ptr : pointers ) >> { >> - if ( ptr.free && ptr.end >= capacity ) >> + if (ptr.free && ptr.getCapacity() >= capacity) >> { >> return ptr; >> } >> @@ -143,7 +143,7 @@ public class OffHeapMemoryBuffer >> } >> } >> >> - final byte[] swp = new byte[pointer.end - pointer.start]; >> + final byte[] swp = new byte[pointer.getCapacity()]; >> buf.get( swp ); >> return swp; >> } >> @@ -158,8 +158,8 @@ public class OffHeapMemoryBuffer >> pointer2free.expiresIn = 0; >> pointer2free.clazz = null; >> pointer2free.directBuffer = null; >> - used.addAndGet( -( pointer2free.end - pointer2free.start ) ); >> - return pointer2free.end - pointer2free.start; >> + used.addAndGet( - pointer2free.getCapacity() ); >> + return pointer2free.getCapacity(); >> } >> >> public void clear() >> @@ -188,8 +188,7 @@ public class OffHeapMemoryBuffer >> if ( goodOne == null ) >> { >> allocationErrors++; >> - throw new NullPointerException( >> - "did not find a suitable buffer " + allocationErrors + " >> times since last cleanup" ); >> + return null; >> } >> >> Pointer fresh = slice( goodOne, payload.length ); >> @@ -353,8 +352,7 @@ public class OffHeapMemoryBuffer >> if ( goodOne == null ) >> { >> allocationErrors++; >> - throw new NullPointerException( >> - "did not find a suitable buffer " + allocationErrors + " >> times since last cleanup" ); >> + return null; >> } >> >> Pointer fresh = slice( goodOne, size ); >> >> Modified: >> incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/Pointer.java >> URL: >> http://svn.apache.org/viewvc/incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/Pointer.java?rev=1227854&r1=1227853&r2=1227854&view=diff >> >> ============================================================================== >> --- >> incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/Pointer.java >> (original) >> +++ >> incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/Pointer.java >> Thu Jan 5 21:46:00 2012 >> @@ -45,6 +45,16 @@ public class Pointer >> >> public ByteBuffer directBuffer = null; >> >> + public Pointer() >> + { >> + } >> + >> + public Pointer( int start, int end ) >> + { >> + this.start = start; >> + this.end = end; >> + } >> + >> public byte[] content() >> { >> return null; >> @@ -66,4 +76,15 @@ public class Pointer >> { >> return (float) ( System.currentTimeMillis() - created ) / hits; >> } >> + >> + public int getCapacity() >> + { >> + return end - start + 1; >> + } >> + >> + @Override >> + public String toString() >> + { >> + return getClass().getSimpleName() + "[" + start + "," + end + "]" >> + ( free ? "" : "not" ) + "free"; >> + } >> } >> >> Added: >> incubator/directmemory/trunk/directmemory-cache/src/test/java/org/apache/directmemory/memory/test/MemoryManagerServiceImplTest.java >> URL: >> http://svn.apache.org/viewvc/incubator/directmemory/trunk/directmemory-cache/src/test/java/org/apache/directmemory/memory/test/MemoryManagerServiceImplTest.java?rev=1227854&view=auto >> >> ============================================================================== >> --- >> incubator/directmemory/trunk/directmemory-cache/src/test/java/org/apache/directmemory/memory/test/MemoryManagerServiceImplTest.java >> (added) >> +++ >> incubator/directmemory/trunk/directmemory-cache/src/test/java/org/apache/directmemory/memory/test/MemoryManagerServiceImplTest.java >> Thu Jan 5 21:46:00 2012 >> @@ -0,0 +1,125 @@ >> +package org.apache.directmemory.memory.test; >> + >> +/* >> + * Licensed to the Apache Software Foundation (ASF) under one >> + * or more contributor license agreements. See the NOTICE file >> + * distributed with this work for additional information >> + * regarding copyright ownership. The ASF licenses this file >> + * to you under the Apache License, Version 2.0 (the >> + * "License"); you may not use this file except in compliance >> + * with the License. You may obtain a copy of the License at >> + * >> + * http://www.apache.org/licenses/LICENSE-2.0 >> + * >> + * Unless required by applicable law or agreed to in writing, >> + * software distributed under the License is distributed on an >> + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY >> + * KIND, either express or implied. See the License for the >> + * specific language governing permissions and limitations >> + * under the License. >> + */ >> + >> +import junit.framework.Assert; >> + >> +import org.apache.directmemory.memory.MemoryManagerServiceImpl; >> +import org.apache.directmemory.memory.Pointer; >> +import org.junit.Test; >> + >> +public class MemoryManagerServiceImplTest { >> + >> + private static final byte[] SMALL_PAYLOAD = "ABCD".getBytes(); >> + >> + @Test >> + public void testFirstMatchBorderCase() { >> + >> + // Storing a first payload of 4 bytes, 1 byte remaining in >> the buffer. When storing a second 4 bytes payload, an >> BufferOverflowException is thrown instead of an easy check. >> + >> + final int BUFFER_SIZE = 5; >> + >> + final MemoryManagerServiceImpl memoryManagerService = new >> MemoryManagerServiceImpl(); >> + >> + memoryManagerService.init(1, BUFFER_SIZE); >> + >> + Pointer pointer1 = >> memoryManagerService.store(SMALL_PAYLOAD); >> + Assert.assertNotNull(pointer1); >> + >> + Pointer pointer2 = >> memoryManagerService.store(SMALL_PAYLOAD); >> + Assert.assertNull(pointer2); >> + >> + } >> + >> + @Test >> + public void testAllocateMultipleBuffers() { >> + >> + // Initializing 4 buffers of 4 bytes, MemoryManagerService >> should search for available space in another buffer. >> + >> + final int NUMBER_OF_OBJECTS = 4; >> + >> + final MemoryManagerServiceImpl memoryManagerService = new >> MemoryManagerServiceImpl(); >> + >> + memoryManagerService.init(NUMBER_OF_OBJECTS, >> SMALL_PAYLOAD.length); >> + >> + for (int i = 0; i < NUMBER_OF_OBJECTS; i++) { >> + Pointer pointer = >> memoryManagerService.store(SMALL_PAYLOAD); >> + Assert.assertNotNull(pointer); >> + } >> + >> + Pointer pointerNull = >> memoryManagerService.store(SMALL_PAYLOAD); >> + Assert.assertNull(pointerNull); >> + } >> + >> + >> + @Test >> + public void testByteLeaking() { >> + >> + // Initializing 1 buffer of 10*4 bytes, should be able to >> allocate 10 objects of 4 bytes. >> + >> + final int NUMBER_OF_OBJECTS = 10; >> + >> + final MemoryManagerServiceImpl memoryManagerService = new >> MemoryManagerServiceImpl(); >> + memoryManagerService.init(1, NUMBER_OF_OBJECTS * >> SMALL_PAYLOAD.length); >> + >> + for (int i = 0; i < NUMBER_OF_OBJECTS; i++) { >> + Pointer pointer = >> memoryManagerService.store(SMALL_PAYLOAD); >> + Assert.assertNotNull(pointer); >> + } >> + >> + Pointer pointerNull = >> memoryManagerService.store(SMALL_PAYLOAD); >> + Assert.assertNull(pointerNull); >> + } >> + >> + >> + @Test >> + public void testReportCorrectUsedMemory() { >> + >> + // Initializing 1 buffer of 4*4 bytes, storing and freeing >> and storing again should report correct numbers. >> + >> + final int NUMBER_OF_OBJECTS = 4; >> + final int BUFFER_SIZE = NUMBER_OF_OBJECTS * >> SMALL_PAYLOAD.length; >> + >> + final MemoryManagerServiceImpl memoryManagerService = new >> MemoryManagerServiceImpl(); >> + >> + memoryManagerService.init(1, BUFFER_SIZE); >> + >> + Pointer lastPointer = null; >> + for (int i = 0; i < NUMBER_OF_OBJECTS; i++) { >> + Pointer pointer = >> memoryManagerService.store(SMALL_PAYLOAD); >> + Assert.assertNotNull(pointer); >> + lastPointer = pointer; >> + } >> + >> + // Buffer is fully used. >> + Assert.assertEquals(BUFFER_SIZE, >> memoryManagerService.getBuffers().get(0).used()); >> + >> + Assert.assertNotNull(lastPointer); >> + memoryManagerService.free( lastPointer ); >> + >> + Pointer pointerNotNull = >> memoryManagerService.store(SMALL_PAYLOAD); >> + Assert.assertNotNull(pointerNotNull); >> + >> + // Buffer again fully used. >> + Assert.assertEquals(BUFFER_SIZE, >> memoryManagerService.getBuffers().get(0).used()); >> + >> + } >> + >> +} >> >> >> -- sent from my Nokia 3210
