Improve read performance when the buffer size is limited

Read notifications should be enabled all the time while the device is
opened unless the buffer size limit is reached. To enable the read
notifier again, the user needs to read the data or enlarge the buffer
with a setReadBufferSize() call.

According to that scenario, it's very likely that the device will
toggle the state of notifier twice (on->off->on) in one notification
cycle. This patch prevents this unnecessary toggling by:

- unconditionally setting the notifier's state in setReadBufferSize()/
  readData();
- deferring the notification disabling in readNotification() on Unix
  and in completeAsyncRead() on Windows to the next notification
  cycle.

Change-Id: I97fc041bb705c034a7e73d1437e64b9b34dc2c18
Reviewed-by: Denis Shienkov <denis.shienkov@gmail.com>

4 files changed, 14 insertions, 25 deletions

diff --git a/src/serialport/qserialport.cpp b/src/serialport/qserialport.cpp
index 65fcfbb..4f9d180 100644
--- a/src/serialport/qserialport.cpp
+++ b/src/serialport/qserialport.cpp
@@ -1240,10 +1240,9 @@ qint64 QSerialPort::readBufferSize() const
 void QSerialPort::setReadBufferSize(qint64 size)
 {
     Q_D(QSerialPort);
-
-    if (d->readBufferMaxSize == size)
-        return;
     d->readBufferMaxSize = size;
+    if (isReadable())
+        d->startAsyncRead();
 }
 
 /*!
@@ -1429,21 +1428,9 @@ qint64 QSerialPort::readData(char *data, qint64 maxSize)
     Q_UNUSED(data);
     Q_UNUSED(maxSize);
 
-#if defined(Q_OS_WIN32)
-    // We need try to start async reading to read a remainder from a driver's queue
-    // in case we have a limited read buffer size. Because the read notification can
-    // be stalled since Windows do not re-triggered an EV_RXCHAR event if a driver's
-    // buffer has a remainder of data ready to read until a new data will be received.
-    Q_D(QSerialPort);
-    if (d->readBufferMaxSize || d->flowControl == QSerialPort::HardwareControl)
-        d->startAsyncRead();
-#elif defined(Q_OS_UNIX)
-    // We need try to re-trigger the read notification to read a remainder from a
-    // driver's queue in case we have a limited read buffer size.
-    Q_D(QSerialPort);
-    if (d->readBufferMaxSize && !d->isReadNotificationEnabled())
-        d->setReadNotificationEnabled(true);
-#endif
+    // In any case we need to start the notifications if they were
+    // disabled by the read handler. If enabled, next call does nothing.
+    d_func()->startAsyncRead();
 
     // return 0 indicating there may be more data in the future
     return qint64(0);
diff --git a/src/serialport/qserialport_p.h b/src/serialport/qserialport_p.h
index a81cc4b..a742e0f 100644
--- a/src/serialport/qserialport_p.h
+++ b/src/serialport/qserialport_p.h
@@ -175,6 +175,8 @@ public:
     bool settingsRestoredOnClose;
     bool isBreakEnabled;
 
+    bool startAsyncRead();
+
 #if defined(Q_OS_WIN32)
 
     bool setDcb(DCB *dcb);
@@ -188,7 +190,6 @@ public:
     bool completeAsyncWrite(qint64 bytesTransferred);
 
     bool startAsyncCommunication();
-    bool startAsyncRead();
     bool _q_startAsyncWrite();
     void _q_notified(DWORD numberOfBytes, DWORD errorCode, OVERLAPPED *overlapped);
 
diff --git a/src/serialport/qserialport_unix.cpp b/src/serialport/qserialport_unix.cpp
index 0c79b47..544658c 100644
--- a/src/serialport/qserialport_unix.cpp
+++ b/src/serialport/qserialport_unix.cpp
@@ -763,6 +763,12 @@ bool QSerialPortPrivate::setFlowControl(QSerialPort::FlowControl flowControl)
     return setTermios(&tio);
 }
 
+bool QSerialPortPrivate::startAsyncRead()
+{
+    setReadNotificationEnabled(true);
+    return true;
+}
+
 bool QSerialPortPrivate::readNotification()
 {
     Q_Q(QSerialPort);
@@ -798,10 +804,6 @@ bool QSerialPortPrivate::readNotification()
 
     newBytes = buffer.size() - newBytes;
 
-    // If read buffer is full, disable the read port notifier.
-    if (readBufferMaxSize && buffer.size() == readBufferMaxSize)
-        setReadNotificationEnabled(false);
-
     // only emit readyRead() when not recursing, and only if there is data available
     const bool hasData = newBytes > 0;
 
diff --git a/src/serialport/qserialport_win.cpp b/src/serialport/qserialport_win.cpp
index 67c68ee..8ca2be9 100644
--- a/src/serialport/qserialport_win.cpp
+++ b/src/serialport/qserialport_win.cpp
@@ -484,8 +484,7 @@ bool QSerialPortPrivate::completeAsyncRead(qint64 bytesTransferred)
     if (bytesTransferred == QSERIALPORT_BUFFERSIZE
             || queuedBytesCount(QSerialPort::Input) > 0) {
         result = startAsyncRead();
-    } else if (readBufferMaxSize == 0
-               || readBufferMaxSize > buffer.size()) {
+    } else {
         result = startAsyncCommunication();
     }