tmf: Improve synchronization in TmfRequestExecutor

If the TmfRequestExecutor is overwhelmed by event requests, it can
freeze the UI thread for long amounts of time and lead to a pretty
bad user experience. (An easy way to test this is to select an entry
with lots of state changes in the CFV, and hold down the right or left
arrow key.)

This patch:
- Makes it use Queue.offer() instead of .add(). This avoids spewing
  exceptions in the console once the queues are full.
- Actually cancels the requests if the offer() fails. Before, even
  though they would not be put in the queue, they would still exist
  and still wait to execute.
- Drastically reduces the size of the request queues. If we are in a
  case where the handler receives requests faster than it can process
  them, instead of giving the user false hope that we will process them
  in a reasonable time, just cancel them so that the UI can go back to
  being responsive faster (see bufferbloat).

The "correct" fix would be to make sure on the request sender's side that
we don't send to many requests, and correctly cancel obsolete ones.
But independently of the sender's implementation, this patch makes the
executor degrade more gracefully.

Change-Id: I0072d54bb7a403773fd4288a42bfb73c4614189b
Reported-by: Simon Marchi <simon.marchi@ericsson.com>
Signed-off-by: Alexandre Montplaisir <alexmonthy@voxpopuli.im>
Reviewed-on: https://git.eclipse.org/r/32115
Tested-by: Hudson CI
Reviewed-by: Matthew Khouzam <matthew.khouzam@ericsson.com>

diff --git a/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/internal/tmf/core/request/TmfRequestExecutor.java b/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/internal/tmf/core/request/TmfRequestExecutor.java
index 96e1c90cd524aaa90939ac48e691797961df301a..a232cabcb2f914f9bf279c46a11bbb1c6e65e97f 100644 (file)
--- a/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/internal/tmf/core/request/TmfRequestExecutor.java
+++ b/org.eclipse.linuxtools.tmf.core/src/org/eclipse/linuxtools/internal/tmf/core/request/TmfRequestExecutor.java
@@ -56,8 +56,8 @@ public class TmfRequestExecutor implements Executor {
     private final String fExecutorName;
 
     // The request queues
-    private final Queue<TmfEventThread> fForegroundTasks = new ArrayBlockingQueue<>(100);
-    private final Queue<TmfEventThread> fBackgroundTasks = new ArrayBlockingQueue<>(100);
+    private final Queue<TmfEventThread> fForegroundTasks = new ArrayBlockingQueue<>(10);
+    private final Queue<TmfEventThread> fBackgroundTasks = new ArrayBlockingQueue<>(10);
 
     // The tasks
     private TmfEventThread fActiveTask;
@@ -89,14 +89,14 @@ public class TmfRequestExecutor implements Executor {
     /**
      * @return the shutdown state (i.e. if it is accepting new requests)
      */
-    public synchronized boolean isShutdown() {
+    public boolean isShutdown() {
         return fExecutor.isShutdown();
     }
 
     /**
      * @return the termination state
      */
-    public synchronized boolean isTerminated() {
+    public boolean isTerminated() {
         return fExecutor.isTerminated();
     }
 
@@ -143,9 +143,13 @@ public class TmfRequestExecutor implements Executor {
         ExecutionType priority = thread.getExecType();
 
         if (priority == ExecutionType.FOREGROUND) {
-            fForegroundTasks.add(wrapper);
+            if (!fForegroundTasks.offer(wrapper)) {
+                wrapper.cancel();
+            }
         } else {
-            fBackgroundTasks.add(wrapper);
+            if (!fBackgroundTasks.offer(wrapper)) {
+                wrapper.cancel();
+            }
         }
     }
 
@@ -177,7 +181,10 @@ public class TmfRequestExecutor implements Executor {
                 } else {
                     if (hasTasks()) {
                         fActiveTask.getThread().suspend();
-                        fForegroundTasks.add(fActiveTask);
+                        if (!fForegroundTasks.offer(fActiveTask)) {
+                            fActiveTask.cancel();
+                            fActiveTask = null;
+                        }
                         schedule();
                     }
                 }
@@ -189,7 +196,10 @@ public class TmfRequestExecutor implements Executor {
                 } else {
                     if (hasTasks()) {
                         fActiveTask.getThread().suspend();
-                        fBackgroundTasks.add(fActiveTask);
+                        if (!fBackgroundTasks.offer(fActiveTask)) {
+                            fActiveTask.cancel();
+                            fActiveTask = null;
+                        }
                         schedule();
                     }
                 }