Merge pull request #217 from geky/socket_event_coalescing

geky · geky · commit a534c20a1745 · 2016-06-07T10:54:02.000+01:00
Add socket event coalescing to reduce cpu usage in lazy implementations
diff --git a/net/NetworkSocketAPI/Socket.cpp b/net/NetworkSocketAPI/Socket.cpp
@@ -23,12 +23,6 @@ Socket::Socket()
 {
 }
 
-Socket::~Socket()
-{
-    // Underlying close is thread safe
-    close();
-}
-
 int Socket::open(NetworkStack *iface, nsapi_protocol_t proto)
 {
     _lock.lock();
@@ -47,7 +41,8 @@ int Socket::open(NetworkStack *iface, nsapi_protocol_t proto)
     }
 
     _socket = socket;
-    _iface->socket_attach(_socket, &Socket::thunk, this);
+    _event.attach(this, &Socket::event);
+    _iface->socket_attach(_socket, Callback<void()>::thunk, &_event);
 
     _lock.unlock();
 
@@ -69,7 +64,7 @@ int Socket::close()
 
     // Wakeup anything in a blocking operation
     // on this socket
-    socket_event();
+    event();
 
     _lock.unlock();
     return ret;
@@ -156,16 +151,3 @@ void Socket::attach(Callback<void()> callback)
 
     _lock.unlock();
 }
-
-void Socket::thunk(void *data)
-{
-    Socket *self = (Socket *)data;
-    self->socket_event();
-}
-
-void Socket::socket_event(void)
-{
-    if (_callback) {
-        _callback();
-    }
-}
diff --git a/net/NetworkSocketAPI/Socket.h b/net/NetworkSocketAPI/Socket.h
@@ -29,7 +29,7 @@ class Socket {
      *
      *  Closes socket if the socket is still open
      */
-    virtual ~Socket();
+    virtual ~Socket() {}
 
     /** Opens a socket
      *
@@ -169,13 +169,12 @@ class Socket {
 protected:
     Socket();
     int open(NetworkStack *iface, nsapi_protocol_t proto);
-
-    static void thunk(void *);
-    virtual void socket_event(void);
+    virtual void event() = 0;
 
     NetworkStack *_iface;
     void *_socket;
     uint32_t _timeout;
+    Callback<void()> _event;
     Callback<void()> _callback;
     rtos::Mutex _lock;
 };
diff --git a/net/NetworkSocketAPI/TCPServer.cpp b/net/NetworkSocketAPI/TCPServer.cpp
@@ -17,15 +17,22 @@
 #include "TCPServer.h"
 #include "Timer.h"
 
-TCPServer::TCPServer(): _accept_sem(0)
+TCPServer::TCPServer()
+    : _pending(0), _accept_sem(0)
 {
 }
 
-TCPServer::TCPServer(NetworkStack *iface): _accept_sem(0)
+TCPServer::TCPServer(NetworkStack *iface)
+    : _pending(0), _accept_sem(0)
 {
     open(iface);
 }
 
+TCPServer::~TCPServer()
+{
+    close();
+}
+
 int TCPServer::open(NetworkStack *iface)
 {
     return Socket::open(iface, NSAPI_TCP);
@@ -55,6 +62,7 @@ int TCPServer::accept(TCPSocket *connection)
             break;
         }
 
+        _pending = 0;
         void *socket;
         ret = _iface->socket_accept(&socket, _socket);
         if (0 == ret) {
@@ -66,7 +74,8 @@ int TCPServer::accept(TCPSocket *connection)
 
             connection->_iface = _iface;
             connection->_socket = socket;
-            _iface->socket_attach(socket, &Socket::thunk, connection);
+            connection->_event = Callback<void()>(connection, &TCPSocket::event);
+            _iface->socket_attach(socket, &Callback<void()>::thunk, &connection->_event);
 
             connection->_lock.unlock();
             break;
@@ -90,12 +99,15 @@ int TCPServer::accept(TCPSocket *connection)
     return ret;
 }
 
-void TCPServer::socket_event()
+void TCPServer::event()
 {
-    int32_t status = _accept_sem.wait(0);
-    if (status <= 1) {
+    int32_t acount = _accept_sem.wait(0);
+    if (acount <= 1) {
         _accept_sem.release();
     }
 
-    Socket::socket_event();
+    _pending += 1;
+    if (_callback && _pending == 1) {
+        _callback();
+    }
 }
diff --git a/net/NetworkSocketAPI/TCPServer.h b/net/NetworkSocketAPI/TCPServer.h
@@ -32,6 +32,12 @@ class TCPServer : public Socket {
      */
     TCPServer();
 
+    /** Destroy a socket
+     *
+     *  Closes socket if the socket is still open
+     */
+    virtual ~TCPServer();
+
     /** Create a socket on a network stack
      *
      *  Creates and opens a socket on the specified network stack.
@@ -76,7 +82,8 @@ class TCPServer : public Socket {
      */
     int accept(TCPSocket *connection);
 protected:
-    virtual void socket_event(void);
+    virtual void event();
+    volatile unsigned _pending;
     rtos::Semaphore _accept_sem;
 };
 
diff --git a/net/NetworkSocketAPI/TCPSocket.cpp b/net/NetworkSocketAPI/TCPSocket.cpp
@@ -17,16 +17,23 @@
 #include "TCPSocket.h"
 #include "Timer.h"
 
-TCPSocket::TCPSocket(): _read_sem(0), _write_sem(0)
+TCPSocket::TCPSocket()
+    : _pending(0), _read_sem(0), _write_sem(0)
 {
 }
 
-TCPSocket::TCPSocket(NetworkStack *iface): _read_sem(0), _write_sem(0)
+TCPSocket::TCPSocket(NetworkStack *iface)
+    : _pending(0), _read_sem(0), _write_sem(0)
 {
     // TCPSocket::open is thread safe
     open(iface);
 }
 
+TCPSocket::~TCPSocket()
+{
+    close();
+}
+
 int TCPSocket::open(NetworkStack *iface)
 {
     // Socket::open is thread safe
@@ -74,6 +81,7 @@ int TCPSocket::send(const void *data, unsigned size)
             break;
         }
 
+        _pending = 0;
         int sent = _iface->socket_send(_socket, data, size);
         if ((0 == _timeout) || (NSAPI_ERROR_WOULD_BLOCK != sent)) {
             ret = sent;
@@ -114,6 +122,7 @@ int TCPSocket::recv(void *data, unsigned size)
             break;
         }
 
+        _pending = 0;
         int recv = _iface->socket_recv(_socket, data, size);
         if ((0 == _timeout) || (NSAPI_ERROR_WOULD_BLOCK != recv)) {
             ret = recv;
@@ -140,17 +149,19 @@ int TCPSocket::recv(void *data, unsigned size)
     return ret;
 }
 
-void TCPSocket::socket_event()
+void TCPSocket::event()
 {
-    int32_t count;
-    count = _write_sem.wait(0);
-    if (count <= 1) {
+    int32_t wcount = _write_sem.wait(0);
+    if (wcount <= 1) {
         _write_sem.release();
     }
-    count = _read_sem.wait(0);
-    if (count <= 1) {
+    int32_t rcount = _read_sem.wait(0);
+    if (rcount <= 1) {
         _read_sem.release();
     }
 
-    Socket::socket_event();
+    _pending += 1;
+    if (_callback && _pending == 1) {
+        _callback();
+    }
 }
diff --git a/net/NetworkSocketAPI/TCPSocket.h b/net/NetworkSocketAPI/TCPSocket.h
@@ -31,6 +31,12 @@ class TCPSocket : public Socket {
      */
     TCPSocket();
 
+    /** Destroy a socket
+     *
+     *  Closes socket if the socket is still open
+     */
+    virtual ~TCPSocket();
+
     /** Create a socket on a network stack
      *
      *  Creates and opens a socket on the specified network stack.
@@ -103,7 +109,8 @@ class TCPSocket : public Socket {
     int recv(void *data, unsigned size);
 
 protected:
-    virtual void socket_event(void);
+    virtual void event();
+    volatile unsigned _pending;
     rtos::Mutex _read_lock;
     rtos::Semaphore _read_sem;
     rtos::Mutex _write_lock;
diff --git a/net/NetworkSocketAPI/UDPSocket.cpp b/net/NetworkSocketAPI/UDPSocket.cpp
@@ -17,15 +17,22 @@
 #include "UDPSocket.h"
 #include "Timer.h"
 
-UDPSocket::UDPSocket(): _read_sem(0), _write_sem(0)
+UDPSocket::UDPSocket()
+    : _pending(0), _read_sem(0), _write_sem(0)
 {
 }
 
-UDPSocket::UDPSocket(NetworkStack *iface): _read_sem(0), _write_sem(0)
+UDPSocket::UDPSocket(NetworkStack *iface)
+    : _pending(0), _read_sem(0), _write_sem(0)
 {
     open(iface);
 }
 
+UDPSocket::~UDPSocket()
+{
+    close();
+}
+
 int UDPSocket::open(NetworkStack *iface)
 {
     return Socket::open(iface, NSAPI_UDP);
@@ -58,6 +65,7 @@ int UDPSocket::sendto(const SocketAddress &address, const void *data, unsigned s
             break;
         }
 
+        _pending = 0;
         int sent = _iface->socket_sendto(_socket, address, data, size);
         if ((0 == _timeout) || (NSAPI_ERROR_WOULD_BLOCK != sent)) {
             ret = sent;
@@ -98,6 +106,7 @@ int UDPSocket::recvfrom(SocketAddress *address, void *buffer, unsigned size)
             break;
         }
 
+        _pending = 0;
         int recv = _iface->socket_recvfrom(_socket, address, buffer, size);
         if ((0 == _timeout) || (NSAPI_ERROR_WOULD_BLOCK != recv)) {
             ret = recv;
@@ -124,17 +133,19 @@ int UDPSocket::recvfrom(SocketAddress *address, void *buffer, unsigned size)
     return ret;
 }
 
-void UDPSocket::socket_event()
+void UDPSocket::event()
 {
-    int32_t count;
-    count = _write_sem.wait(0);
-    if (count <= 1) {
+    int32_t wcount = _write_sem.wait(0);
+    if (wcount <= 1) {
         _write_sem.release();
     }
-    count = _read_sem.wait(0);
-    if (count <= 1) {
+    int32_t rcount = _read_sem.wait(0);
+    if (rcount <= 1) {
         _read_sem.release();
     }
 
-    Socket::socket_event();
+    _pending += 1;
+    if (_callback && _pending == 1) {
+        _callback();
+    }
 }
diff --git a/net/NetworkSocketAPI/UDPSocket.h b/net/NetworkSocketAPI/UDPSocket.h
@@ -31,6 +31,12 @@ class UDPSocket : public Socket {
      */
     UDPSocket();
 
+    /** Destroy a socket
+     *
+     *  Closes socket if the socket is still open
+     */
+    virtual ~UDPSocket();
+
     /** Create a socket on a network stack
      *
      *  Creates and opens a socket on the specified network stack.
@@ -102,7 +108,8 @@ class UDPSocket : public Socket {
      */
     int recvfrom(SocketAddress *address, void *data, unsigned size);
 protected:
-    virtual void socket_event(void);
+    virtual void event();
+    volatile unsigned _pending;
     rtos::Mutex _read_lock;
     rtos::Semaphore _read_sem;
     rtos::Mutex _write_lock;

Original file line number	Diff line number	Diff line change
`@@ -17,15 +17,22 @@`
`17`	`17`	`#include "TCPServer.h"`
`18`	`18`	`#include "Timer.h"`
`19`	`19`
`20`		`-TCPServer::TCPServer(): _accept_sem(0)`
	`20`	`+TCPServer::TCPServer()`
	`21`	`+ : _pending(0), _accept_sem(0)`
`21`	`22`	`{`
`22`	`23`	`}`
`23`	`24`
`24`		`-TCPServer::TCPServer(NetworkStack *iface): _accept_sem(0)`
	`25`	`+TCPServer::TCPServer(NetworkStack *iface)`
	`26`	`+ : _pending(0), _accept_sem(0)`
`25`	`27`	`{`
`26`	`28`	`open(iface);`
`27`	`29`	`}`
`28`	`30`
	`31`	`+TCPServer::~TCPServer()`
	`32`	`+{`
	`33`	`+ close();`
	`34`	`+}`
	`35`	`+`
`29`	`36`	`int TCPServer::open(NetworkStack *iface)`
`30`	`37`	`{`
`31`	`38`	`return Socket::open(iface, NSAPI_TCP);`
`@@ -55,6 +62,7 @@ int TCPServer::accept(TCPSocket *connection)`
`55`	`62`	`break;`
`56`	`63`	`}`
`57`	`64`
	`65`	`+ _pending = 0;`
`58`	`66`	`void *socket;`
`59`	`67`	`ret = _iface->socket_accept(&socket, _socket);`
`60`	`68`	`if (0 == ret) {`
`@@ -66,7 +74,8 @@ int TCPServer::accept(TCPSocket *connection)`
`66`	`74`
`67`	`75`	`connection->_iface = _iface;`
`68`	`76`	`connection->_socket = socket;`
`69`		`- _iface->socket_attach(socket, &Socket::thunk, connection);`
	`77`	`+ connection->_event = Callback<void()>(connection, &TCPSocket::event);`
	`78`	`+ _iface->socket_attach(socket, &Callback<void()>::thunk, &connection->_event);`
`70`	`79`
`71`	`80`	`connection->_lock.unlock();`
`72`	`81`	`break;`
`@@ -90,12 +99,15 @@ int TCPServer::accept(TCPSocket *connection)`
`90`	`99`	`return ret;`
`91`	`100`	`}`
`92`	`101`
`93`		`-void TCPServer::socket_event()`
	`102`	`+void TCPServer::event()`
`94`	`103`	`{`
`95`		`- int32_t status = _accept_sem.wait(0);`
`96`		`- if (status <= 1) {`
	`104`	`+ int32_t acount = _accept_sem.wait(0);`
	`105`	`+ if (acount <= 1) {`
`97`	`106`	`_accept_sem.release();`
`98`	`107`	`}`
`99`	`108`
`100`		`- Socket::socket_event();`
	`109`	`+ _pending += 1;`
	`110`	`+ if (_callback && _pending == 1) {`
	`111`	`+ _callback();`
	`112`	`+ }`
`101`	`113`	`}`
Original file line number	Diff line number	Diff line change
`@@ -17,16 +17,23 @@`
`17`	`17`	`#include "TCPSocket.h"`
`18`	`18`	`#include "Timer.h"`
`19`	`19`
`20`		`-TCPSocket::TCPSocket(): _read_sem(0), _write_sem(0)`
	`20`	`+TCPSocket::TCPSocket()`
	`21`	`+ : _pending(0), _read_sem(0), _write_sem(0)`
`21`	`22`	`{`
`22`	`23`	`}`
`23`	`24`
`24`		`-TCPSocket::TCPSocket(NetworkStack *iface): _read_sem(0), _write_sem(0)`
	`25`	`+TCPSocket::TCPSocket(NetworkStack *iface)`
	`26`	`+ : _pending(0), _read_sem(0), _write_sem(0)`
`25`	`27`	`{`
`26`	`28`	`// TCPSocket::open is thread safe`
`27`	`29`	`open(iface);`
`28`	`30`	`}`
`29`	`31`
	`32`	`+TCPSocket::~TCPSocket()`
	`33`	`+{`
	`34`	`+ close();`
	`35`	`+}`
	`36`	`+`
`30`	`37`	`int TCPSocket::open(NetworkStack *iface)`
`31`	`38`	`{`
`32`	`39`	`// Socket::open is thread safe`
`@@ -74,6 +81,7 @@ int TCPSocket::send(const void *data, unsigned size)`
`74`	`81`	`break;`
`75`	`82`	`}`
`76`	`83`
	`84`	`+ _pending = 0;`
`77`	`85`	`int sent = _iface->socket_send(_socket, data, size);`
`78`	`86`	`if ((0 == _timeout) \|\| (NSAPI_ERROR_WOULD_BLOCK != sent)) {`
`79`	`87`	`ret = sent;`
`@@ -114,6 +122,7 @@ int TCPSocket::recv(void *data, unsigned size)`
`114`	`122`	`break;`
`115`	`123`	`}`
`116`	`124`
	`125`	`+ _pending = 0;`
`117`	`126`	`int recv = _iface->socket_recv(_socket, data, size);`
`118`	`127`	`if ((0 == _timeout) \|\| (NSAPI_ERROR_WOULD_BLOCK != recv)) {`
`119`	`128`	`ret = recv;`
`@@ -140,17 +149,19 @@ int TCPSocket::recv(void *data, unsigned size)`
`140`	`149`	`return ret;`
`141`	`150`	`}`
`142`	`151`
`143`		`-void TCPSocket::socket_event()`
	`152`	`+void TCPSocket::event()`
`144`	`153`	`{`
`145`		`- int32_t count;`
`146`		`- count = _write_sem.wait(0);`
`147`		`- if (count <= 1) {`
	`154`	`+ int32_t wcount = _write_sem.wait(0);`
	`155`	`+ if (wcount <= 1) {`
`148`	`156`	`_write_sem.release();`
`149`	`157`	`}`
`150`		`- count = _read_sem.wait(0);`
`151`		`- if (count <= 1) {`
	`158`	`+ int32_t rcount = _read_sem.wait(0);`
	`159`	`+ if (rcount <= 1) {`
`152`	`160`	`_read_sem.release();`
`153`	`161`	`}`
`154`	`162`
`155`		`- Socket::socket_event();`
	`163`	`+ _pending += 1;`
	`164`	`+ if (_callback && _pending == 1) {`
	`165`	`+ _callback();`
	`166`	`+ }`
`156`	`167`	`}`