BELUGABEHR commented on a change in pull request #869: ZOOKEEPER-3020: Review 
of SyncRequestProcessor
URL: https://github.com/apache/zookeeper/pull/869#discussion_r270133326
 
 

 ##########
 File path: 
zookeeper-server/src/main/java/org/apache/zookeeper/server/SyncRequestProcessor.java
 ##########
 @@ -46,36 +50,38 @@
  */
 public class SyncRequestProcessor extends ZooKeeperCriticalThread implements
         RequestProcessor {
+
     private static final Logger LOG = 
LoggerFactory.getLogger(SyncRequestProcessor.class);
+
+    /** The number of log entries to log before starting a snapshot */
+    private static int snapCount = ZooKeeperServer.getSnapCount();
+
+    private static final Request REQUEST_OF_DEATH = Request.requestOfDeath;
+
+    private final static int FLUSH_SIZE = 1000;
+
     private final ZooKeeperServer zks;
-    private final LinkedBlockingQueue<Request> queuedRequests =
+
+    private final BlockingQueue<Request> queuedRequests =
         new LinkedBlockingQueue<Request>();
+
     private final RequestProcessor nextProcessor;
 
-    private Thread snapInProcess = null;
-    volatile private boolean running;
+    private final Semaphore snapThreadMutex = new Semaphore(1);
 
     /**
      * Transactions that have been written and are waiting to be flushed to
      * disk. Basically this is the list of SyncItems whose callbacks will be
      * invoked after flush returns successfully.
      */
-    private final LinkedList<Request> toFlush = new LinkedList<Request>();
-    private final Random r = new Random();
-    /**
-     * The number of log entries to log before starting a snapshot
-     */
-    private static int snapCount = ZooKeeperServer.getSnapCount();
-
-    private final Request requestOfDeath = Request.requestOfDeath;
+    private final Queue<Request> toFlush = new ArrayDeque<>(FLUSH_SIZE);
 
 Review comment:
   > This class is likely to be faster than Stack when used as a stack, and 
faster than LinkedList when used as a queue.
   
   https://docs.oracle.com/javase/8/docs/api/java/util/ArrayDeque.html
   
   And it will definitely win out here since the max size is known.  Think 
about it... every time an item is inserted into the LinkedList, a new Node 
needs to be instantiated and then the links need to be updated in the data 
structure.  With the array-based implementation, at initialization, it is 
created with an internal buffer of size 1000.  Any items added to the Queue are 
simply dropped into the next available bucket in the array.  There is no 
instantiations or any other interactions with the JDK memory manager.  Also, 
when iterating over the queue, all of the links in the LinkedList must be 
walked.  In the array-based implementation, it's a simple array iteration in 
memory that is all local (where as LinkedList nodes can be all over the place). 
 Finally, when removing items, the LinkedList links must again be updated and 
the LinkedList node must be cleaned up by the garbage collection.  ArrayDeque 
simply removes the item by setting its bucket in the array to 'null'.

----------------------------------------------------------------
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:
[email protected]


With regards,
Apache Git Services

Reply via email to