You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@directmemory.apache.org by "Benoit Perroud (Created) (JIRA)" <ji...@apache.org> on 2012/02/24 10:53:48 UTC

[jira] [Created] (DIRECTMEMORY-70) Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry

Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry
---------------------------------------------------------------------------------

                 Key: DIRECTMEMORY-70
                 URL: https://issues.apache.org/jira/browse/DIRECTMEMORY-70
             Project: Apache DirectMemory
          Issue Type: Improvement
            Reporter: Benoit Perroud


In order to achieve better separation of responsabilities, attributes like Pointer.class, Pointer.hits, Pointer.lastHit, etc. should be moved into an intermediate class, for instance CacheEntry.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (DIRECTMEMORY-70) Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry

Posted by "Simone Tripodi (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DIRECTMEMORY-70?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13219177#comment-13219177 ] 

Simone Tripodi commented on DIRECTMEMORY-70:
--------------------------------------------

Salut!

let's take in consideration that shielding from “data races" is different to keeping the data structure thread safe - anyway if you are confident that the used locking level is robust enough to keep the data structure thread-safe, I trust you.
Anyway if you'll need to traverse the structure, take in consideration the SkipList.

merci a toi,
-Simo
                
> Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry
> ---------------------------------------------------------------------------------
>
>                 Key: DIRECTMEMORY-70
>                 URL: https://issues.apache.org/jira/browse/DIRECTMEMORY-70
>             Project: Apache DirectMemory
>          Issue Type: Improvement
>            Reporter: Benoit Perroud
>         Attachments: DIRECTMEMORY-70-MemoryManagerService-to-hide-implementation.patch, DIRECTMEMORY-70-byte-bufer-allocator.patch, DIRECTMEMORY-70-byte-bufer-allocator.patch, DIRECTMEMORY-72-byteBufferAllocator-v2.patch
>
>
> In order to achieve better separation of responsabilities, attributes like Pointer.class, Pointer.hits, Pointer.lastHit, etc. should be moved into an intermediate class, for instance CacheEntry.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

       

[jira] [Updated] (DIRECTMEMORY-70) Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry

Posted by "Benoit Perroud (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DIRECTMEMORY-70?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Benoit Perroud updated DIRECTMEMORY-70:
---------------------------------------

    Attachment: DIRECTMEMORY-70-byte-bufer-allocator.patch

ByteBufferAllocators and theirs integration into MemoryManagerService
                
> Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry
> ---------------------------------------------------------------------------------
>
>                 Key: DIRECTMEMORY-70
>                 URL: https://issues.apache.org/jira/browse/DIRECTMEMORY-70
>             Project: Apache DirectMemory
>          Issue Type: Improvement
>            Reporter: Benoit Perroud
>         Attachments: DIRECTMEMORY-70-MemoryManagerService-to-hide-implementation.patch, DIRECTMEMORY-70-byte-bufer-allocator.patch, DIRECTMEMORY-70-byte-bufer-allocator.patch
>
>
> In order to achieve better separation of responsabilities, attributes like Pointer.class, Pointer.hits, Pointer.lastHit, etc. should be moved into an intermediate class, for instance CacheEntry.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (DIRECTMEMORY-70) Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry

Posted by "Simone Tripodi (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DIRECTMEMORY-70?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13219124#comment-13219124 ] 

Simone Tripodi commented on DIRECTMEMORY-70:
--------------------------------------------

Salut Benoit,

I just had a look at the latest patch, looks good and I don't see any reason to hold on and keeping uncommitted, so if there is no objection shown by anybody, please do! :)

Just few minor questions:

{code}
+public class SlabByteBufferAllocatorImpl
...
+    private final TreeMap<Integer, FixedSizeByteBufferAllocatorImpl> slabs = new TreeMap<Integer, FixedSizeByteBufferAllocatorImpl>();
{code}

this is smart indeed and doesn't require sort operations when needed, anyway I would suggest you to have a look at the [ConcurrentSkipListMap|http://docs.oracle.com/javase/6/docs/api/java/util/concurrent/ConcurrentSkipListMap.html] not sure how concurrency is required here, didn't investigate so deep, but anyway the SkipList has the advantage respect to the Binary tree of performing the "get me the next" in O(1) against O(log n) of the TreeMap.
Same for {{{MergingByteBufferAllocatorImpl}}}

{code}
+    @Override
+    public int getCapacity()
+    {
+        int totalSize = 0;
+        for (final Map.Entry<Integer, FixedSizeByteBufferAllocatorImpl> entry : slabs.entrySet())
+        {
+            totalSize += entry.getValue().getCapacity();
+        }
+        return totalSize;
+    }
{code}

this operation has a non trivial O(n) cost - I'd expect off-heap cache will store fantazillions of elements - can't we switch over keeping an accumulator class member, to calculate the size every time an element is put/deleted?

{code}
+    private static class LinkedByteBuffer
{code}

NICE, I like it! :)

{code}
+public class FixedSizeByteBufferAllocatorImpl
...
+    private final Queue<ByteBuffer> freeBuffers = new ConcurrentLinkedQueue<ByteBuffer>();
{code}

IIUC here you just mind the order of queuing, right?

{code}
+        Preconditions.checkArgument( byteBuffer.capacity() == sliceSize );
{code}

trivial, but IMHO importing {{Preconditions.checkArgument}} statically looks better (same thing for junit's Assert)

{code}
 public class MemoryManagerServiceImpl<V>
..
+    private final List<ByteBufferAllocator> allocators = new ArrayList<ByteBufferAllocator>();
{code}

I'd move to a {{{LinkedList}}} as concrete implementation, because, always under the conditions that an OffHeap cache should contain zillions of elements, ArrayList is subjected to be often resized (operation that has a non trivial cost, at runtime)


Just to remark this, in order to increase performances, we have to understand how to get rid of the Guava iterators I used to replace JOQL, they are not efficient:

{code}
+        Iterable<Pointer<V>> result = from( new Comparator<Pointer<V>>()
+        {
+
+            public int compare( Pointer<V> o1, Pointer<V> o2 )
+            {
+                float f1 = o1.getFrequency();
+                float f2 = o2.getFrequency();
+
+                return Float.compare( f1, f2 );
+            }
+
+        } ).sortedCopy( limit( filter( pointers, new Predicate<Pointer<V>>()
+        {
+
+            @Override
+            public boolean apply( Pointer<V> input )
+            {
+                return !input.isFree();
+            }
+
+        } ), limit ) );
+
+        free( result );
{code}

if the {{{pointers}}} could be replaced by a {{{NavigableMap}}} implementation we can save the cost of sorting, and the {{{free}}} method should be invoked as soon as an element in the data structure matches, not collecting first and free them in a second time (wich has the iteration cost)

+1 to apply the patch, to your discretion following my suggestions or not. Well done and congrats, good job! :)
                
> Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry
> ---------------------------------------------------------------------------------
>
>                 Key: DIRECTMEMORY-70
>                 URL: https://issues.apache.org/jira/browse/DIRECTMEMORY-70
>             Project: Apache DirectMemory
>          Issue Type: Improvement
>            Reporter: Benoit Perroud
>         Attachments: DIRECTMEMORY-70-MemoryManagerService-to-hide-implementation.patch, DIRECTMEMORY-70-byte-bufer-allocator.patch, DIRECTMEMORY-70-byte-bufer-allocator.patch, DIRECTMEMORY-72-byteBufferAllocator-v2.patch
>
>
> In order to achieve better separation of responsabilities, attributes like Pointer.class, Pointer.hits, Pointer.lastHit, etc. should be moved into an intermediate class, for instance CacheEntry.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (DIRECTMEMORY-70) Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry

Posted by "Benoit Perroud (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DIRECTMEMORY-70?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Benoit Perroud updated DIRECTMEMORY-70:
---------------------------------------

    Attachment: DIRECTMEMORY-72-byteBufferAllocator-v2.patch

More work on ByteBufferAllocator interface. I intentionnally ommited the tests to reduce burden. 
This new interface is intended to replace the OffHeapMemoryBuffer with a more simple interface and more self-explaining name.
100% backward compatible, all tests are passing.
                
> Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry
> ---------------------------------------------------------------------------------
>
>                 Key: DIRECTMEMORY-70
>                 URL: https://issues.apache.org/jira/browse/DIRECTMEMORY-70
>             Project: Apache DirectMemory
>          Issue Type: Improvement
>            Reporter: Benoit Perroud
>         Attachments: DIRECTMEMORY-70-MemoryManagerService-to-hide-implementation.patch, DIRECTMEMORY-70-byte-bufer-allocator.patch, DIRECTMEMORY-70-byte-bufer-allocator.patch, DIRECTMEMORY-72-byteBufferAllocator-v2.patch
>
>
> In order to achieve better separation of responsabilities, attributes like Pointer.class, Pointer.hits, Pointer.lastHit, etc. should be moved into an intermediate class, for instance CacheEntry.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (DIRECTMEMORY-70) Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry

Posted by "Simone Tripodi (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DIRECTMEMORY-70?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13215666#comment-13215666 ] 

Simone Tripodi commented on DIRECTMEMORY-70:
--------------------------------------------

please show how it would become in your fork!!! :)
                
> Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry
> ---------------------------------------------------------------------------------
>
>                 Key: DIRECTMEMORY-70
>                 URL: https://issues.apache.org/jira/browse/DIRECTMEMORY-70
>             Project: Apache DirectMemory
>          Issue Type: Improvement
>            Reporter: Benoit Perroud
>
> In order to achieve better separation of responsabilities, attributes like Pointer.class, Pointer.hits, Pointer.lastHit, etc. should be moved into an intermediate class, for instance CacheEntry.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (DIRECTMEMORY-70) Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry

Posted by "Benoit Perroud (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DIRECTMEMORY-70?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Benoit Perroud updated DIRECTMEMORY-70:
---------------------------------------

    Attachment: DIRECTMEMORY-70-byte-bufer-allocator.patch

ByteBufferAllocators and their integration into MemoryManagerService (ASF granted)
                
> Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry
> ---------------------------------------------------------------------------------
>
>                 Key: DIRECTMEMORY-70
>                 URL: https://issues.apache.org/jira/browse/DIRECTMEMORY-70
>             Project: Apache DirectMemory
>          Issue Type: Improvement
>            Reporter: Benoit Perroud
>         Attachments: DIRECTMEMORY-70-MemoryManagerService-to-hide-implementation.patch, DIRECTMEMORY-70-byte-bufer-allocator.patch, DIRECTMEMORY-70-byte-bufer-allocator.patch
>
>
> In order to achieve better separation of responsabilities, attributes like Pointer.class, Pointer.hits, Pointer.lastHit, etc. should be moved into an intermediate class, for instance CacheEntry.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (DIRECTMEMORY-70) Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry

Posted by "Simone Tripodi (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DIRECTMEMORY-70?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13217143#comment-13217143 ] 

Simone Tripodi commented on DIRECTMEMORY-70:
--------------------------------------------

Salut Benoit,
sounds interesting, could you share please a patch where showing the complete refactor? Just to have a look at the overall design!
Merci en avance,
-Simo
                
> Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry
> ---------------------------------------------------------------------------------
>
>                 Key: DIRECTMEMORY-70
>                 URL: https://issues.apache.org/jira/browse/DIRECTMEMORY-70
>             Project: Apache DirectMemory
>          Issue Type: Improvement
>            Reporter: Benoit Perroud
>         Attachments: DIRECTMEMORY-70-MemoryManagerService-to-hide-implementation.patch
>
>
> In order to achieve better separation of responsabilities, attributes like Pointer.class, Pointer.hits, Pointer.lastHit, etc. should be moved into an intermediate class, for instance CacheEntry.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (DIRECTMEMORY-70) Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry

Posted by "Simone Tripodi (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DIRECTMEMORY-70?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13215551#comment-13215551 ] 

Simone Tripodi commented on DIRECTMEMORY-70:
--------------------------------------------

Then {{Pointer}} would be a member of {{CacheEntry}}? Or you have a different design in mind?
TIA!
                
> Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry
> ---------------------------------------------------------------------------------
>
>                 Key: DIRECTMEMORY-70
>                 URL: https://issues.apache.org/jira/browse/DIRECTMEMORY-70
>             Project: Apache DirectMemory
>          Issue Type: Improvement
>            Reporter: Benoit Perroud
>
> In order to achieve better separation of responsabilities, attributes like Pointer.class, Pointer.hits, Pointer.lastHit, etc. should be moved into an intermediate class, for instance CacheEntry.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (DIRECTMEMORY-70) Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry

Posted by "Hudson (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DIRECTMEMORY-70?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13220871#comment-13220871 ] 

Hudson commented on DIRECTMEMORY-70:
------------------------------------

Integrated in directmemory-trunk #164 (See [https://builds.apache.org/job/directmemory-trunk/164/])
    DIRECTMEMORY-40, DIRECTMEMORY-70 : augment testing, fix bug when clearing allocator and freeing slab buffer (Revision 1296133)

     Result = UNSTABLE
bperroud : 
Files : 
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/MemoryManagerServiceImpl.java
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/allocator/AbstractByteBufferAllocator.java
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/allocator/FixedSizeByteBufferAllocatorImpl.java
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/allocator/MergingByteBufferAllocatorImpl.java
* /incubator/directmemory/trunk/directmemory-cache/src/test/java/org/apache/directmemory/memory/AbstractMemoryManagerServiceTest.java
* /incubator/directmemory/trunk/directmemory-cache/src/test/java/org/apache/directmemory/memory/DefaultMemoryManagerServiceTest.java
* /incubator/directmemory/trunk/directmemory-cache/src/test/java/org/apache/directmemory/memory/SlabMemoryManagerServiceTest.java

                
> Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry
> ---------------------------------------------------------------------------------
>
>                 Key: DIRECTMEMORY-70
>                 URL: https://issues.apache.org/jira/browse/DIRECTMEMORY-70
>             Project: Apache DirectMemory
>          Issue Type: Improvement
>            Reporter: Benoit Perroud
>         Attachments: DIRECTMEMORY-70-MemoryManagerService-to-hide-implementation.patch, DIRECTMEMORY-70-byte-bufer-allocator.patch, DIRECTMEMORY-70-byte-bufer-allocator.patch, DIRECTMEMORY-72-byteBufferAllocator-v2.patch
>
>
> In order to achieve better separation of responsabilities, attributes like Pointer.class, Pointer.hits, Pointer.lastHit, etc. should be moved into an intermediate class, for instance CacheEntry.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (DIRECTMEMORY-70) Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry

Posted by "Benoit Perroud (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DIRECTMEMORY-70?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13216757#comment-13216757 ] 

Benoit Perroud commented on DIRECTMEMORY-70:
--------------------------------------------

And {{MemoryManagerService}}.capacity(), used(), ... should also be in a {{Statistics}} class.
                
> Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry
> ---------------------------------------------------------------------------------
>
>                 Key: DIRECTMEMORY-70
>                 URL: https://issues.apache.org/jira/browse/DIRECTMEMORY-70
>             Project: Apache DirectMemory
>          Issue Type: Improvement
>            Reporter: Benoit Perroud
>         Attachments: DIRECTMEMORY-70-MemoryManagerService-to-hide-implementation.patch
>
>
> In order to achieve better separation of responsabilities, attributes like Pointer.class, Pointer.hits, Pointer.lastHit, etc. should be moved into an intermediate class, for instance CacheEntry.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (DIRECTMEMORY-70) Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry

Posted by "Benoit Perroud (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DIRECTMEMORY-70?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13217132#comment-13217132 ] 

Benoit Perroud commented on DIRECTMEMORY-70:
--------------------------------------------

Just to ensure I'm not completely off road, I extracted the following simplier interface to replace the {{OffHeapMemoryBuffer}}

{{code}}
public interface ByteBufferAllocator
{
    
    /**
     * Allocate the given size off heap, or return null if the allocation failed.
     * @param size
     * @return
     */
    ByteBuffer allocate( final int size );

    /**
     * Return the given ByteBuffer making it available for a future usage 
     * @param buffer
     */
    void free( final ByteBuffer buffer );

}
{{/code}}

This is lean and easy to understand. Two implementations are (almost) ready to be used : a merging one, i.e. where returned pointers are merged together, and a slab'style allocator, where buffers are of the same predefined sizes (i.e. 128b, 512b, 2k, 8k, 32k,... application specific).

{{MemoryManagerService}} is a more high level view with existing store, retrieve, free and "collectExpire" functions, and wrap the allocated {{ByteBuffer}} in a {{Pointer}} and return it.

{{MemoryManagerService}} is augmented with Input and OutputStream functionalities to be able to stream in files, socket data, etc.

WDYT ?

                
> Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry
> ---------------------------------------------------------------------------------
>
>                 Key: DIRECTMEMORY-70
>                 URL: https://issues.apache.org/jira/browse/DIRECTMEMORY-70
>             Project: Apache DirectMemory
>          Issue Type: Improvement
>            Reporter: Benoit Perroud
>         Attachments: DIRECTMEMORY-70-MemoryManagerService-to-hide-implementation.patch
>
>
> In order to achieve better separation of responsabilities, attributes like Pointer.class, Pointer.hits, Pointer.lastHit, etc. should be moved into an intermediate class, for instance CacheEntry.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (DIRECTMEMORY-70) Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry

Posted by "Benoit Perroud (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DIRECTMEMORY-70?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13215646#comment-13215646 ] 

Benoit Perroud commented on DIRECTMEMORY-70:
--------------------------------------------

Yes you got it, {{Pointer}} would be a member of {{CacheEntry}}.

Then the Cache layer will deal with CacheEntries, and the OffHeapMemory layer will deal with Pointers only.
And the Cache layer will be responsible for eviction, not the OffHeapMemory layer.
                
> Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry
> ---------------------------------------------------------------------------------
>
>                 Key: DIRECTMEMORY-70
>                 URL: https://issues.apache.org/jira/browse/DIRECTMEMORY-70
>             Project: Apache DirectMemory
>          Issue Type: Improvement
>            Reporter: Benoit Perroud
>
> In order to achieve better separation of responsabilities, attributes like Pointer.class, Pointer.hits, Pointer.lastHit, etc. should be moved into an intermediate class, for instance CacheEntry.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Updated] (DIRECTMEMORY-70) Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry

Posted by "Benoit Perroud (Updated) (JIRA)" <ji...@apache.org>.
     [ https://issues.apache.org/jira/browse/DIRECTMEMORY-70?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Benoit Perroud updated DIRECTMEMORY-70:
---------------------------------------

    Attachment: DIRECTMEMORY-70-MemoryManagerService-to-hide-implementation.patch

First step : MemoryManagerService to hide internal implementation -> removing getBuffers() and getActiveBuffer from the interface.
                
> Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry
> ---------------------------------------------------------------------------------
>
>                 Key: DIRECTMEMORY-70
>                 URL: https://issues.apache.org/jira/browse/DIRECTMEMORY-70
>             Project: Apache DirectMemory
>          Issue Type: Improvement
>            Reporter: Benoit Perroud
>         Attachments: DIRECTMEMORY-70-MemoryManagerService-to-hide-implementation.patch
>
>
> In order to achieve better separation of responsabilities, attributes like Pointer.class, Pointer.hits, Pointer.lastHit, etc. should be moved into an intermediate class, for instance CacheEntry.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (DIRECTMEMORY-70) Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry

Posted by "Benoit Perroud (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DIRECTMEMORY-70?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13219132#comment-13219132 ] 

Benoit Perroud commented on DIRECTMEMORY-70:
--------------------------------------------

Thanks a lot for the review !

Answering your comments : 

The {{{TreeMap}}} usage is safe here because the map is initialized at initialization, then only read. On {{{MergingByteBufferAllocatorImpl}}} the {{{TreeMap}}} manipulations are all done inside a {{{Lock}}} guard, so should also be fine.

I will rework the {{{getCapacity()}}} to reduce it's computational cost.

The {{{ConcurrentLinkedQueue}}} is just used as a repository for free buffers. We don't care about ordering, sorting or other, we just want to put a free buffer, and retrieve it later. Here a {{{LinkedList}}} could fit but concurrency is expected, I didn't find a lighter structure than this one.

The {{{ArrayList}} used in {{{MemoryManagerServiceImpl}}} is because we know the size of the list, and we need index access. I just did it wrong, thanks for pointing this out :) Note that buffers are not stored in this structure, but in the {{ConcurrentLinkedQueue}} so no zillion entries here.

Regarding Guava iteration, I just copy-pasted the existing one, but I agree we should rework on this. I will open a ticket for this point.

Thanks again for your time !

                
> Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry
> ---------------------------------------------------------------------------------
>
>                 Key: DIRECTMEMORY-70
>                 URL: https://issues.apache.org/jira/browse/DIRECTMEMORY-70
>             Project: Apache DirectMemory
>          Issue Type: Improvement
>            Reporter: Benoit Perroud
>         Attachments: DIRECTMEMORY-70-MemoryManagerService-to-hide-implementation.patch, DIRECTMEMORY-70-byte-bufer-allocator.patch, DIRECTMEMORY-70-byte-bufer-allocator.patch, DIRECTMEMORY-72-byteBufferAllocator-v2.patch
>
>
> In order to achieve better separation of responsabilities, attributes like Pointer.class, Pointer.hits, Pointer.lastHit, etc. should be moved into an intermediate class, for instance CacheEntry.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (DIRECTMEMORY-70) Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry

Posted by "Hudson (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DIRECTMEMORY-70?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13220603#comment-13220603 ] 

Hudson commented on DIRECTMEMORY-70:
------------------------------------

Integrated in directmemory-windows #20 (See [https://builds.apache.org/job/directmemory-windows/20/])
    DIRECTMEMORY-40, DIRECTMEMORY-70 : separate responsabilities more clearly, set allocation with merging pointers as the default one, add a SLAB's style allocator (fixed buffer size), mark OffHeapMemoryBuffer as deprecated (Revision 1295522)

     Result = FAILURE
bperroud : 
Files : 
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/cache/CacheServiceImpl.java
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/AllocationPolicy.java
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/MemoryManager.java
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/MemoryManagerService.java
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/MemoryManagerServiceImpl.java
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/MemoryManagerServiceWithAllocationPolicyImpl.java
* /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/OffHeapMemoryBufferImpl.java
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/OffHeapMergingMemoryBufferImpl.java
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/PointerImpl.java
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/RoundRobinAllocationPolicy.java
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/allocator
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/allocator/AbstractByteBufferAllocator.java
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/allocator/ByteBufferAllocator.java
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/allocator/DirectByteBufferUtils.java
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/allocator/FixedSizeByteBufferAllocatorImpl.java
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/allocator/MergingByteBufferAllocatorImpl.java
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/allocator/SlabByteBufferAllocatorImpl.java
* /incubator/directmemory/trunk/directmemory-cache/src/test/java/org/apache/directmemory/cache/CacheServiceImplTest.java
* /incubator/directmemory/trunk/directmemory-cache/src/test/java/org/apache/directmemory/memory/AbstractOffHeapMemoryBufferTest.java
* /incubator/directmemory/trunk/directmemory-cache/src/test/java/org/apache/directmemory/memory/Concurrent2Test.java
* /incubator/directmemory/trunk/directmemory-cache/src/test/java/org/apache/directmemory/memory/Concurrent3Test.java
* /incubator/directmemory/trunk/directmemory-cache/src/test/java/org/apache/directmemory/memory/MemoryManagerServiceImplTest.java
* /incubator/directmemory/trunk/directmemory-cache/src/test/java/org/apache/directmemory/memory/MemoryManagerServiceImplWithMerginOHMBAndAllocationPolicyTest.java
* /incubator/directmemory/trunk/directmemory-cache/src/test/java/org/apache/directmemory/memory/MemoryManagerServiceImplWithMerginOHMBTest.java
* /incubator/directmemory/trunk/directmemory-cache/src/test/java/org/apache/directmemory/memory/MemoryManagerTest.java
* /incubator/directmemory/trunk/directmemory-cache/src/test/java/org/apache/directmemory/memory/OffHeapMergingMemoryBufferTest.java
* /incubator/directmemory/trunk/directmemory-cache/src/test/java/org/apache/directmemory/memory/RoundRobinAllocationPolicyTest.java
* /incubator/directmemory/trunk/directmemory-cache/src/test/java/org/apache/directmemory/memory/Starter.java
* /incubator/directmemory/trunk/directmemory-cache/src/test/java/org/apache/directmemory/memory/allocator
* /incubator/directmemory/trunk/directmemory-cache/src/test/java/org/apache/directmemory/memory/allocator/FixedSizeByteBufferAllocatorImplTest.java
* /incubator/directmemory/trunk/directmemory-cache/src/test/java/org/apache/directmemory/memory/allocator/MergingByteBufferAllocatorImplTest.java
* /incubator/directmemory/trunk/directmemory-cache/src/test/java/org/apache/directmemory/memory/allocator/SlabByteBufferAllocatorImplTest.java
* /incubator/directmemory/trunk/integrations/ehcache/src/main/java/org/apache/directmemory/ehcache/DirectMemoryCache.java
* /incubator/directmemory/trunk/integrations/ehcache/src/main/java/org/apache/directmemory/ehcache/DirectMemoryStore.java
* /incubator/directmemory/trunk/integrations/ehcache/src/test/java/org/apache/directmemory/ehcache/EHCacheTest.java
* /incubator/directmemory/trunk/itests/osgi/src/test/java/org/apache/directmemory/tests/osgi/cache/CacheServiceExportingActivator.java

                
> Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry
> ---------------------------------------------------------------------------------
>
>                 Key: DIRECTMEMORY-70
>                 URL: https://issues.apache.org/jira/browse/DIRECTMEMORY-70
>             Project: Apache DirectMemory
>          Issue Type: Improvement
>            Reporter: Benoit Perroud
>         Attachments: DIRECTMEMORY-70-MemoryManagerService-to-hide-implementation.patch, DIRECTMEMORY-70-byte-bufer-allocator.patch, DIRECTMEMORY-70-byte-bufer-allocator.patch, DIRECTMEMORY-72-byteBufferAllocator-v2.patch
>
>
> In order to achieve better separation of responsabilities, attributes like Pointer.class, Pointer.hits, Pointer.lastHit, etc. should be moved into an intermediate class, for instance CacheEntry.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

[jira] [Commented] (DIRECTMEMORY-70) Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry

Posted by "Hudson (Commented) (JIRA)" <ji...@apache.org>.
    [ https://issues.apache.org/jira/browse/DIRECTMEMORY-70?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13220788#comment-13220788 ] 

Hudson commented on DIRECTMEMORY-70:
------------------------------------

Integrated in directmemory-trunk #162 (See [https://builds.apache.org/job/directmemory-trunk/162/])
    DIRECTMEMORY-40, DIRECTMEMORY-70 : add javadoc and comments, put logger in abstract, add feature to close an allocator and release all the held buffers. (Revision 1296090)

     Result = SUCCESS
bperroud : 
Files : 
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/MemoryManagerServiceImpl.java
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/allocator/AbstractByteBufferAllocator.java
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/allocator/ByteBufferAllocator.java
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/allocator/DirectByteBufferUtils.java
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/allocator/FixedSizeByteBufferAllocatorImpl.java
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/allocator/MergingByteBufferAllocatorImpl.java
* /incubator/directmemory/trunk/directmemory-cache/src/main/java/org/apache/directmemory/memory/allocator/SlabByteBufferAllocatorImpl.java

                
> Move Pointer.class and Pointer."statistics" into an intermediate class CacheEntry
> ---------------------------------------------------------------------------------
>
>                 Key: DIRECTMEMORY-70
>                 URL: https://issues.apache.org/jira/browse/DIRECTMEMORY-70
>             Project: Apache DirectMemory
>          Issue Type: Improvement
>            Reporter: Benoit Perroud
>         Attachments: DIRECTMEMORY-70-MemoryManagerService-to-hide-implementation.patch, DIRECTMEMORY-70-byte-bufer-allocator.patch, DIRECTMEMORY-70-byte-bufer-allocator.patch, DIRECTMEMORY-72-byteBufferAllocator-v2.patch
>
>
> In order to achieve better separation of responsabilities, attributes like Pointer.class, Pointer.hits, Pointer.lastHit, etc. should be moved into an intermediate class, for instance CacheEntry.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira