incubator-directmemory-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Maurizio Cucchiara <maurizio.cucchi...@gmail.com>
Subject Re: svn commit: r1227854 - in /incubator/directmemory/trunk/directmemory-cache/src: main/java/org/apache/directmemory/memory/ test/java/org/apache/directmemory/memory/test/
Date Thu, 05 Jan 2012 22:37:04 GMT
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, <bperroud@apache.org> 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());
> +
> +       }
> +
> +}
>
>
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message