[GitHub] [activemq-artemis] franz1981 commented on a change in pull request #3303: ARTEMIS-2949 Reduce GC on OperationContext::checkTasks

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

franz1981 commented on a change in pull request #3303:
URL: https://github.com/apache/activemq-artemis/pull/3303#discussion_r508516563



##########
File path: artemis-server/src/main/java/org/apache/activemq/artemis/core/persistence/impl/journal/OperationContextImpl.java
##########
@@ -191,41 +192,75 @@ public void executeOnCompletion(final IOCallback completion, final boolean store
 
    }
 
+   private boolean validateTasksAdd(int storeLined, int replicationLined, int pageLined) {
+      if (tasks.isEmpty()) {
+         return true;
+      }
+      final TaskHolder holder = tasks.peekLast();
+      if (holder.storeLined > storeLined ||
+         holder.replicationLined > replicationLined ||
+         holder.pageLined > pageLined) {
+         return false;
+      }
+      return true;
+   }
+
    @Override
    public synchronized void done() {
       stored++;
       checkTasks();
    }
 
+   private void checkStoreTasks() {
+      final LinkedList<StoreTaskHolder> storeOnlyTasks = this.storeOnlyTasks;
+      assert storeOnlyTasks != null;
+      final int size = storeOnlyTasks.size();
+      if (size == 0) {
+         return;
+      }
+      final long stored = this.stored;
+      for (int i = 0; i < size; i++) {
+         final StoreTaskHolder holder = storeOnlyTasks.peek();
+         if (holder.storeLined < stored) {
+            // fail fast: storeOnlyTasks are ordered by storeLined, there is no need to continue
+            return;
+         }
+         // If set, we use an executor to avoid the server being single threaded
+         execute(holder.task);
+         final StoreTaskHolder removed = storeOnlyTasks.poll();
+         assert removed == holder;
+      }
+   }
+
+   private void checkGenericTasks() {
+      final LinkedList<TaskHolder> tasks = this.tasks;
+      assert tasks != null;
+      final int size = this.tasks.size();
+      if (size == 0) {
+         return;
+      }
+      assert size >= 1;
+      // no need to use an iterator here, we can save that cost
+      for (int i = 0; i < size; i++) {
+         final TaskHolder holder = tasks.peek();
+         if (stored < holder.storeLined || replicated < holder.replicationLined || paged < holder.pageLined) {
+            // End of list here. No other task will be completed after this
+            return;
+         }
+         execute(holder.task);
+         final TaskHolder removed = tasks.poll();
+         assert removed == holder;
+      }
+   }
+
    private void checkTasks() {
 
       if (storeOnlyTasks != null) {
-         Iterator<TaskHolder> iter = storeOnlyTasks.iterator();
-         while (iter.hasNext()) {
-            TaskHolder holder = iter.next();
-            if (stored >= holder.storeLined) {
-               // If set, we use an executor to avoid the server being single threaded
-               execute(holder.task);
-
-               iter.remove();
-            }
-         }
+         checkStoreTasks();
       }
 
       if (stored >= minimalStore && replicated >= minimalReplicated && paged >= minimalPage) {
-         Iterator<TaskHolder> iter = tasks.iterator();
-         while (iter.hasNext()) {
-            TaskHolder holder = iter.next();
-            if (stored >= holder.storeLined && replicated >= holder.replicationLined && paged >= holder.pageLined) {
-               // If set, we use an executor to avoid the server being single threaded
-               execute(holder.task);
-
-               iter.remove();
-            } else {
-               // End of list here. No other task will be completed after this
-               break;
-            }
-         }
+         checkGenericTasks();

Review comment:
       :+1: 




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to 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