From 68a82a3eb9a4ab957927cc75c192f1c84b1e7eaf Mon Sep 17 00:00:00 2001 From: Sean T Allen Date: Mon, 17 Jun 2019 23:32:29 -0400 Subject: [PATCH] Simplify buffering in TCPConnection Prior to this commit, the buffering in TCPConnection was overly complicated. This was a carry over from the old style of buffer handling. However, after Dipin Hora's commits to bring over the "faster tcp" changes from Wallaroo, the buffer handling no longer makes sense. Prior to this commit, a TCPConnection would be provided two variables to control read buffering: init_size, max_size. The read buffer would start at init_size and would grow to max_size. Before "faster tcp", the buffer would only grow in size if we read the entire buffer's worth of data in one recv call. After the data was read, the buffer would be truncated and then handed off to the TCPConnectionNotify. This was very wasteful. If you had a buffer of 128 allocated bytes and only read 4, then the 124 allocated bytes would be truncated away and the buffer would be reallocated. With "faster tcp", we allocate a large buffer and when reading data, we chop it off the buffer as needed. This means, that "grow the buffer" doesn't make much sense anymore. The buffer will continually shrink in size as we slowly chop more and more off. This meant that with the code as it is prior to change, we would always grow to max buffer size. If we are always going to grow to max buffer size, it makes sense to simplify the code and our interfaces and only allow for setting the buffer size. This commit makes that simplication. Now we will: - allocate a buffer of size `read_buffer_size` - reallocate when the buffer is empty - reallocate if the caller is using `expect` and the buffer is too small to hold the expected payload This is a breaking change for TPCConnection and TCPListener as it changes the constructor interfaces. --- CHANGELOG.md | 1 + packages/net/tcp_connection.pony | 64 +++++++++++--------------------- packages/net/tcp_listener.pony | 25 +++++-------- 3 files changed, 31 insertions(+), 59 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 78c167eccca..54d67fca74e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ All notable changes to the Pony compiler and standard library will be documented - Make TCPConnection yield on writes to not hog cpu ([PR #3176](https://github.com/ponylang/ponyc/pull/3176)) - Change how TCP connection reads data to improve performance ([PR #3178](https://github.com/ponylang/ponyc/pull/3178)) - Allow use of runtime TCP connect without ASIO one shot ([PR #3171](https://github.com/ponylang/ponyc/pull/3171)) +- Simplify buffering in TCPConnection ([PR #3185](https://github.com/ponylang/ponyc/pull/3185)) ## [0.28.1] - 2019-06-01 diff --git a/packages/net/tcp_connection.pony b/packages/net/tcp_connection.pony index d97a4e0537b..46161d40fd4 100644 --- a/packages/net/tcp_connection.pony +++ b/packages/net/tcp_connection.pony @@ -214,8 +214,7 @@ actor TCPConnection var _read_buf_offset: USize = 0 var _max_received_called: USize = 50 - var _next_size: USize - let _max_size: USize + let _read_buffer_size: USize var _expect: USize = 0 @@ -227,16 +226,14 @@ actor TCPConnection host: String, service: String, from: String = "", - init_size: USize = 64, - max_size: USize = 16384) + read_buffer_size: USize = 16384) => """ Connect via IPv4 or IPv6. If `from` is a non-empty string, the connection will be made from the specified interface. """ - _read_buf = recover Array[U8] .> undefined(init_size) end - _next_size = init_size - _max_size = max_size + _read_buf = recover Array[U8] .> undefined(read_buffer_size) end + _read_buffer_size = read_buffer_size _notify = consume notify let asio_flags = ifdef not windows then @@ -255,15 +252,13 @@ actor TCPConnection host: String, service: String, from: String = "", - init_size: USize = 64, - max_size: USize = 16384) + read_buffer_size: USize = 16384) => """ Connect via IPv4. """ - _read_buf = recover Array[U8] .> undefined(init_size) end - _next_size = init_size - _max_size = max_size + _read_buf = recover Array[U8] .> undefined(read_buffer_size) end + _read_buffer_size = read_buffer_size _notify = consume notify let asio_flags = ifdef not windows then @@ -282,15 +277,13 @@ actor TCPConnection host: String, service: String, from: String = "", - init_size: USize = 64, - max_size: USize = 16384) + read_buffer_size: USize = 16384) => """ Connect via IPv6. """ - _read_buf = recover Array[U8] .> undefined(init_size) end - _next_size = init_size - _max_size = max_size + _read_buf = recover Array[U8] .> undefined(read_buffer_size) end + _read_buffer_size = read_buffer_size _notify = consume notify let asio_flags = ifdef not windows then @@ -307,8 +300,7 @@ actor TCPConnection listen: TCPListener, notify: TCPConnectionNotify iso, fd: U32, - init_size: USize = 64, - max_size: USize = 16384) + read_buffer_size: USize = 16384) => """ A new connection accepted on a server. @@ -329,9 +321,8 @@ actor TCPConnection @pony_asio_event_set_writeable(_event, true) end _writeable = true - _read_buf = recover Array[U8] .> undefined(init_size) end - _next_size = init_size - _max_size = max_size + _read_buf = recover Array[U8] .> undefined(read_buffer_size) end + _read_buffer_size = read_buffer_size _notify.accepted(this) @@ -657,7 +648,7 @@ actor TCPConnection var bytes_to_send: USize = 0 var bytes_sent: USize = 0 while _writeable and (_pending_writev_total > 0) do - if bytes_sent >= _max_size then + if bytes_sent >= _read_buffer_size then _write_again() return false end @@ -775,16 +766,13 @@ actor TCPConnection This occurs only with IOCP on Windows. """ ifdef windows then - match len.usize() - | 0 => + if len.usize() == 0 then // The socket has been closed from the other side, or a hard close has // cancelled the queued read. _readable = false _shutdown_peer = true close() return - | _next_size => - _next_size = _max_size.min(_next_size * 2) end _read_buf_offset = _read_buf_offset + len.usize() @@ -806,16 +794,10 @@ actor TCPConnection fun ref _read_buf_size() => """ - Resize the read buffer. + Resize the read buffer if it is empty or smaller than the next payload size """ - if _expect != 0 then - // We know how much data to expect. If _read_buf is too small, - // then resize it. - if (_read_buf.size() - _read_buf_offset) <= _expect then - _read_buf.undefined(_expect.next_pow2().max(_next_size)) - end - else - _read_buf.undefined(_next_size) + if _read_buf.size() <= _expect then + _read_buf.undefined(_read_buffer_size) end fun ref _queue_read() => @@ -879,8 +861,8 @@ actor TCPConnection end end - if sum >= _max_size then - // If we've read _max_size, yield and read again later. + if sum >= _read_buffer_size then + // If we've read _read_buffer_size, yield and read again later. _read_buf_size() _read_again() _reading = false @@ -898,8 +880,7 @@ actor TCPConnection _read_buf.cpointer(_read_buf_offset), _read_buf.size() - _read_buf_offset) ? - match len - | 0 => + if len == 0 then // Would block, try again later. // this is safe because asio thread isn't currently subscribed // for a read event so will not be writing to the readable flag @@ -908,9 +889,6 @@ actor TCPConnection _reading = false @pony_asio_event_resubscribe_read(_event) return - | (_read_buf.size() - _read_buf_offset) => - // Increase the read buffer size. - _next_size = _max_size.min(_next_size * 2) end _read_buf_offset = _read_buf_offset + len diff --git a/packages/net/tcp_listener.pony b/packages/net/tcp_listener.pony index 3135bc3d718..37aba549e42 100644 --- a/packages/net/tcp_listener.pony +++ b/packages/net/tcp_listener.pony @@ -45,8 +45,7 @@ actor TCPListener let _limit: USize var _count: USize = 0 var _paused: Bool = false - var _init_size: USize - var _max_size: USize + var _read_buffer_size: USize new create( auth: TCPListenerAuth, @@ -54,8 +53,7 @@ actor TCPListener host: String = "", service: String = "0", limit: USize = 0, - init_size: USize = 64, - max_size: USize = 16384) + read_buffer_size: USize = 16384) => """ Listens for both IPv4 and IPv6 connections. @@ -65,8 +63,7 @@ actor TCPListener _event = @pony_os_listen_tcp[AsioEventID](this, host.cstring(), service.cstring()) - _init_size = init_size - _max_size = max_size + _read_buffer_size = read_buffer_size _fd = @pony_asio_event_fd(_event) _notify_listening() @@ -76,8 +73,7 @@ actor TCPListener host: String = "", service: String = "0", limit: USize = 0, - init_size: USize = 64, - max_size: USize = 16384) + read_buffer_size: USize = 16384) => """ Listens for IPv4 connections. @@ -87,8 +83,7 @@ actor TCPListener _event = @pony_os_listen_tcp4[AsioEventID](this, host.cstring(), service.cstring()) - _init_size = init_size - _max_size = max_size + _read_buffer_size = read_buffer_size _fd = @pony_asio_event_fd(_event) _notify_listening() @@ -98,8 +93,7 @@ actor TCPListener host: String = "", service: String = "0", limit: USize = 0, - init_size: USize = 64, - max_size: USize = 16384) + read_buffer_size: USize = 16384) => """ Listens for IPv6 connections. @@ -109,8 +103,7 @@ actor TCPListener _event = @pony_os_listen_tcp6[AsioEventID](this, host.cstring(), service.cstring()) - _init_size = init_size - _max_size = max_size + _read_buffer_size = read_buffer_size _fd = @pony_asio_event_fd(_event) _notify_listening() @@ -217,8 +210,8 @@ actor TCPListener Spawn a new connection. """ try - TCPConnection._accept(this, _notify.connected(this)?, ns, _init_size, - _max_size) + TCPConnection._accept(this, _notify.connected(this)?, ns, + _read_buffer_size) _count = _count + 1 else @pony_os_socket_close[None](ns)