[GitHub] franz1981 commented on a change in pull request #2494: ARTEMIS-2224 Reduce contention on LivePageCacheImpl

[GitBox <gi...@apache.org>]

franz1981 commented on a change in pull request #2494: ARTEMIS-2224 Reduce contention on LivePageCacheImpl
URL: https://github.com/apache/activemq-artemis/pull/2494#discussion_r246602031
 
 

 ##########
 File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/paging/cursor/impl/LivePageCacheImpl.java
 ##########
 @@ -48,54 +82,228 @@ public long getPageId() {
    }
 
    @Override
-   public synchronized int getNumberOfMessages() {
-      return messages.size();
+   public int getNumberOfMessages() {
+      while (true) {
+         final long size = producerIndex;
+         if (size == RESIZING) {
+            Thread.yield();
+            continue;
+         }
+         return (int) Math.min(size, Integer.MAX_VALUE);
+      }
    }
 
    @Override
-   public synchronized void setMessages(PagedMessage[] messages) {
+   public void setMessages(PagedMessage[] messages) {
       // This method shouldn't be called on liveCache, but we will provide the implementation for it anyway
       for (PagedMessage msg : messages) {
          addLiveMessage(msg);
       }
    }
 
    @Override
-   public synchronized PagedMessage getMessage(int messageNumber) {
-      if (messageNumber < messages.size()) {
-         return messages.get(messageNumber);
-      } else {
+   public PagedMessage getMessage(int messageNumber) {
+      if (messageNumber < 0) {
          return null;
       }
+      //it allow to perform less cache invalidations vs producerIndex if there are bursts of appends
+      long size = lastSeenProducerIndex;
+      if (messageNumber >= size) {
+         while ((size = producerIndex) == RESIZING) {
+            Thread.yield();
+         }
+         //it is a message over the current size?
+         if (messageNumber >= size) {
 
 Review comment:
   Is not that easy here, this one is not a common collection (in theory, but we can get an agreement on the contract for sure): it is more an append only list that allows indexed queries like a map. In addition, is concurrent so if you try to query something that's not in the collection in that moment, but you will do it right after, there is a chance that you can get it if it has been added in parallel by another thread.
   If I remember correctly it is similar to an [hashed array tree](https://en.wikipedia.org/wiki/Hashed_array_tree), where the top-level directory is a double linked list of "folders" (instead of an array, like the original implementation): indeed in the code there are chunkIndex (==key of top level directory) and offset (==key of leaf into a directory).

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services