From 0e0eb354527cecdc22d1c0e6ecba06d7f747e728 Mon Sep 17 00:00:00 2001
From: David Reiss <dreiss@apache.org>
Date: Wed, 6 Oct 2010 17:10:29 +0000
Subject: [PATCH] THRIFT-926. cpp: Don't sleep in TFileTransport if we have
 data to write

Previously, the TFileTransport writer thread behaved as follows:

  while true:
    wait for main thread to notify new data in enqueueBuffer_
    swap(enqueueBuffer_, dequeueBuffer_)
    write out everything in dequeueBuffer_

Now the behavior is:

  while true:
    if enqueueBuffer_ is empty
      wait for main thread to notify new data in enqueueBuffer_
    swap(enqueueBuffer_, dequeueBuffer_)
    write out everything in dequeueBuffer_

The old behavior had a couple problems:
- Writes that arrived while the writer thread was writing
  dequeueBuffer_ wouldn't get processed immediately.  The writer thread
  would always wait until another write occurred after it started its
  condition variable wait, or until it timed out (3 seconds by default).

- If the main thread was writing fast enough to fill up enqueueBuffer_
  while the writer thread was still writing out dequeueBuffer_, it would
  block the next write call until the writer thread swapped the buffers.
  Unfortunately, the writer thread waits to do this until it the main
  thread notifies it of another write.  This deadlock is only broken by
  the 3 second timeout.  Performance then tanks, since the writer thread
  now always sleeps 3 seconds each time around the loop.

git-svn-id: https://svn.apache.org/repos/asf/incubator/thrift/trunk@1005153 13f79535-47bb-0310-9956-ffa450edef68
---
 lib/cpp/src/transport/TFileTransport.cpp | 30 ++++++++++++++----------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/lib/cpp/src/transport/TFileTransport.cpp b/lib/cpp/src/transport/TFileTransport.cpp
index ff69431e..7ad4167a 100644
--- a/lib/cpp/src/transport/TFileTransport.cpp
+++ b/lib/cpp/src/transport/TFileTransport.cpp
@@ -272,33 +272,37 @@ void TFileTransport::enqueueEvent(const uint8_t* buf, uint32_t eventLen, bool bl
 
 bool TFileTransport::swapEventBuffers(struct timespec* deadline) {
   pthread_mutex_lock(&mutex_);
-  if (deadline != NULL) {
-    // if we were handed a deadline time struct, do a timed wait
-    pthread_cond_timedwait(&notEmpty_, &mutex_, deadline);
+
+  bool swap;
+  if (!enqueueBuffer_->isEmpty()) {
+    swap = true;
   } else {
-    // just wait until the buffer gets an item
-    pthread_cond_wait(&notEmpty_, &mutex_);
-  }
+    if (deadline != NULL) {
+      // if we were handed a deadline time struct, do a timed wait
+      pthread_cond_timedwait(&notEmpty_, &mutex_, deadline);
+    } else {
+      // just wait until the buffer gets an item
+      pthread_cond_wait(&notEmpty_, &mutex_);
+    }
 
-  bool swapped = false;
+    // could be empty if we timed out
+    swap = enqueueBuffer_->isEmpty();
+  }
 
-  // could be empty if we timed out
-  if (!enqueueBuffer_->isEmpty()) {
+  if (swap) {
     TFileTransportBuffer *temp = enqueueBuffer_;
     enqueueBuffer_ = dequeueBuffer_;
     dequeueBuffer_ = temp;
-
-    swapped = true;
   }
 
   // unlock the mutex and signal if required
   pthread_mutex_unlock(&mutex_);
 
-  if (swapped) {
+  if (swap) {
     pthread_cond_signal(&notFull_);
   }
 
-  return swapped;
+  return swap;
 }
 
 
-- 
2.17.1