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()); > + > + } > + > +} > > >
