Optimisation - buffer packet read in pendingDatagramSize

In Symbian, the OS function to get the size of a pending datagram also
includes the size of the packet header (which is different for IPv4 and
IPv6). We were reading the datagram with the "peek" flag set to
implement pendingDatagramSize, then reading again normally when the
client called read/readDatagram.

This change removes the "peek" flag, and buffers the datagram in the
socket engine, returning it and clearing the buffer when read or
readDatagram is called.
If there is no buffered data, the existing code path is followed - it
isn't mandatory to call pendingDatagramSize before reading from the
socket.

Reviewed-by: Markus Goetz

2 files changed, 56 insertions, 9 deletions

diff --git a/src/network/socket/qsymbiansocketengine.cpp b/src/network/socket/qsymbiansocketengine.cpp
index b2b655e..b6d12fe 100644
--- a/src/network/socket/qsymbiansocketengine.cpp
+++ b/src/network/socket/qsymbiansocketengine.cpp
@@ -251,7 +251,8 @@ QSymbianSocketEnginePrivate::QSymbianSocketEnginePrivate() :
     readNotificationsEnabled(false),
     writeNotificationsEnabled(false),
     exceptNotificationsEnabled(false),
-    asyncSelect(0)
+    asyncSelect(0),
+    hasReceivedBufferedDatagram(false)
 {
 }
 
@@ -781,21 +782,34 @@ qint64 QSymbianSocketEngine::pendingDatagramSize() const
     Q_D(const QSymbianSocketEngine);
     Q_CHECK_VALID_SOCKETLAYER(QSymbianSocketEngine::pendingDatagramSize(), false);
     Q_CHECK_TYPE(QSymbianSocketEngine::hasPendingDatagrams(), QAbstractSocket::UdpSocket, false);
-    int nbytes;
+    //can only buffer one datagram at a time
+    if (d->hasReceivedBufferedDatagram)
+        return d->receivedDataBuffer.size();
+    int nbytes = 0;
     TInt err = d->nativeSocket.GetOpt(KSOReadBytesPending,KSOLSocket, nbytes);
     if (nbytes > 0) {
         //nbytes includes IP header, which is of variable length (IPv4 with or without options, IPv6...)
-        QByteArray next(nbytes,0);
-        TPtr8 buffer((TUint8*)next.data(), next.size());
+        //therefore read the datagram into a buffer to find its true size
+        d->receivedDataBuffer.resize(nbytes);
+        TPtr8 buffer((TUint8*)d->receivedDataBuffer.data(), nbytes);
+        //nbytes = size including IP header, buffer is a pointer descriptor backed by the receivedDataBuffer
         TInetAddr addr;
         TRequestStatus status;
-        //TODO: rather than peek, should we save this for next call to readDatagram?
-        //what if calls don't match though?
-        d->nativeSocket.RecvFrom(buffer, addr, KSockReadPeek, status);
+        //RecvFrom copies only the payload (we don't want the header so don't specify the option to retrieve it)
+        d->nativeSocket.RecvFrom(buffer, addr, 0, status);
         User::WaitForRequest(status);
-        if (status.Int())
+        if (status != KErrNone) {
+            d->receivedDataBuffer.clear();
             return 0;
-        return buffer.Length();
+        }
+        nbytes = buffer.Length();
+        //nbytes = size of payload, resize the receivedDataBuffer to the final size
+        d->receivedDataBuffer.resize(nbytes);
+        d->hasReceivedBufferedDatagram = true;
+        //now receivedDataBuffer contains one datagram, which has been removed from the socket's internal buffer
+#if defined (QNATIVESOCKETENGINE_DEBUG)
+        qDebug() << "QSymbianSocketEngine::pendingDatagramSize buffering" << nbytes << "bytes";
+#endif
     }
     return qint64(nbytes);
 }
@@ -807,6 +821,19 @@ qint64 QSymbianSocketEngine::readDatagram(char *data, qint64 maxSize,
     Q_D(QSymbianSocketEngine);
     Q_CHECK_VALID_SOCKETLAYER(QSymbianSocketEngine::readDatagram(), -1);
     Q_CHECK_TYPE(QSymbianSocketEngine::readDatagram(), QAbstractSocket::UdpSocket, false);
+
+    // if a datagram was buffered in pendingDatagramSize(), return it now
+    if (d->hasReceivedBufferedDatagram) {
+        qint64 size = qMin(maxSize, (qint64)d->receivedDataBuffer.size());
+        memcpy(data, d->receivedDataBuffer.constData(), size);
+        d->receivedDataBuffer.clear();
+        d->hasReceivedBufferedDatagram = false;
+#if defined (QNATIVESOCKETENGINE_DEBUG)
+        qDebug() << "QSymbianSocketEngine::readDatagram returning" << size << "bytes from buffer";
+#endif
+        return size;
+    }
+
     TPtr8 buffer((TUint8*)data, (int)maxSize);
     TInetAddr addr;
     TRequestStatus status;
@@ -985,6 +1012,9 @@ void QSymbianSocketEngine::close()
     d->localAddress.clear();
     d->peerPort = 0;
     d->peerAddress.clear();
+
+    d->hasReceivedBufferedDatagram = false;
+    d->receivedDataBuffer.clear();
 }
 
 qint64 QSymbianSocketEngine::write(const char *data, qint64 len)
@@ -1036,6 +1066,18 @@ qint64 QSymbianSocketEngine::read(char *data, qint64 maxSize)
     Q_CHECK_VALID_SOCKETLAYER(QSymbianSocketEngine::read(), -1);
     Q_CHECK_STATES(QSymbianSocketEngine::read(), QAbstractSocket::ConnectedState, QAbstractSocket::BoundState, -1);
 
+    // if a datagram was buffered in pendingDatagramSize(), return it now
+    if (d->hasReceivedBufferedDatagram) {
+        qint64 size = qMin(maxSize, (qint64)d->receivedDataBuffer.size());
+        memcpy(data, d->receivedDataBuffer.constData(), size);
+        d->receivedDataBuffer.clear();
+        d->hasReceivedBufferedDatagram = false;
+#if defined (QNATIVESOCKETENGINE_DEBUG)
+        qDebug() << "QSymbianSocketEngine::read returning" << size << "bytes from buffer";
+#endif
+        return size;
+    }
+
     TPtr8 buffer((TUint8*)data, (int)maxSize);
     TSockXfrLength received = 0;
     TRequestStatus status;
@@ -1655,6 +1697,9 @@ void QAsyncSelect::run()
     //when event loop disabled socket events, defer until later
     if (maybeDeferSocketEvent())
         return;
+#if defined (QNATIVESOCKETENGINE_DEBUG)
+    qDebug() << "QAsyncSelect::run" << m_selectBuf() << m_selectFlags;
+#endif
     m_inSocketEvent = true;
     m_selectBuf() &= m_selectFlags; //the select ioctl reports everything, so mask to only what we requested
     //KSockSelectReadContinuation is for reading datagrams in a mode that doesn't discard when the
diff --git a/src/network/socket/qsymbiansocketengine_p.h b/src/network/socket/qsymbiansocketengine_p.h
index 85ab54a..2e7c155 100644
--- a/src/network/socket/qsymbiansocketengine_p.h
+++ b/src/network/socket/qsymbiansocketengine_p.h
@@ -196,6 +196,8 @@ public:
     bool exceptNotificationsEnabled;
     QAsyncSelect* asyncSelect;
 
+    mutable QByteArray receivedDataBuffer;
+    mutable bool hasReceivedBufferedDatagram;
     // FIXME this is duplicated from qnativesocketengine_p.h
     enum ErrorString {
         NonBlockingInitFailedErrorString,