From 2bdd6e227c309622684eb4a0073a89a1f49e86ea Mon Sep 17 00:00:00 2001 From: James M Snell Date: Fri, 26 Jun 2020 11:36:05 -0700 Subject: [PATCH 1/2] quic: minor cleanups in quic_buffer --- src/quic/node_quic_buffer-inl.h | 1 - src/quic/node_quic_buffer.cc | 2 +- src/quic/node_quic_buffer.h | 12 ++++++------ 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/src/quic/node_quic_buffer-inl.h b/src/quic/node_quic_buffer-inl.h index e03378a331154b..25f130ac9fea60 100644 --- a/src/quic/node_quic_buffer-inl.h +++ b/src/quic/node_quic_buffer-inl.h @@ -64,7 +64,6 @@ QuicBuffer::QuicBuffer(QuicBuffer&& src) noexcept src.ended_ = false; } - QuicBuffer& QuicBuffer::operator=(QuicBuffer&& src) noexcept { if (this == &src) return *this; this->~QuicBuffer(); diff --git a/src/quic/node_quic_buffer.cc b/src/quic/node_quic_buffer.cc index 2cb9211994138d..d3762589c84127 100644 --- a/src/quic/node_quic_buffer.cc +++ b/src/quic/node_quic_buffer.cc @@ -17,7 +17,7 @@ void QuicBufferChunk::MemoryInfo(MemoryTracker* tracker) const { size_t QuicBuffer::Push(uv_buf_t* bufs, size_t nbufs, DoneCB done) { size_t len = 0; - if (nbufs == 0 || bufs == nullptr || is_empty(bufs[0])) { + if (UNLIKELY(nbufs == 0 || bufs == nullptr || is_empty(bufs[0]))) { done(0); return 0; } diff --git a/src/quic/node_quic_buffer.h b/src/quic/node_quic_buffer.h index 17f59a7e759161..3d81a28176dd51 100644 --- a/src/quic/node_quic_buffer.h +++ b/src/quic/node_quic_buffer.h @@ -24,7 +24,7 @@ using DoneCB = std::function; // When data is sent over QUIC, we are required to retain it in memory // until we receive an acknowledgement that it has been successfully -// acknowledged. The QuicBuffer object is what we use to handle that +// received. The QuicBuffer object is what we use to handle that // and track until it is acknowledged. To understand the QuicBuffer // object itself, it is important to understand how ngtcp2 and nghttp3 // handle data that is given to it to serialize into QUIC packets. @@ -52,7 +52,7 @@ using DoneCB = std::function; // QuicBuffer is further complicated by design quirks and limitations // of the StreamBase API and how it interacts with the JavaScript side. // -// QuicBuffer is essentially a linked list of QuicBufferChunk instances. +// QuicBuffer is a linked list of QuicBufferChunk instances. // A single QuicBufferChunk wraps a single non-zero-length uv_buf_t. // When the QuicBufferChunk is created, we capture the total length // of the buffer and the total number of bytes remaining to be sent. @@ -79,7 +79,7 @@ using DoneCB = std::function; // along with a callback to be called when the data has // been consumed. // -// Any given chunk as a remaining-to-be-acknowledged length (length()) and a +// Any given chunk has a remaining-to-be-acknowledged length (length()) and a // remaining-to-be-read-length (remaining()). The former tracks the number // of bytes that have yet to be acknowledged by the QUIC peer. Once the // remaining-to-be-acknowledged length reaches zero, the done callback @@ -88,7 +88,7 @@ using DoneCB = std::function; // serialized into QUIC packets and sent. // The remaining-to-be-acknowledged length is adjusted using consume(), // while the remaining-to-be-ead length is adjusted using seek(). -class QuicBufferChunk : public MemoryRetainer { +class QuicBufferChunk final : public MemoryRetainer { public: // Default non-op done handler. static void default_done(int status) {} @@ -149,8 +149,8 @@ class QuicBufferChunk : public MemoryRetainer { friend class QuicBuffer; }; -class QuicBuffer : public bob::SourceImpl, - public MemoryRetainer { +class QuicBuffer final : public bob::SourceImpl, + public MemoryRetainer { public: QuicBuffer() = default; From 03a10841c7206ac8ee18ac6fa42532d7a8caecb1 Mon Sep 17 00:00:00 2001 From: James M Snell Date: Sat, 27 Jun 2020 09:12:11 -0700 Subject: [PATCH 2/2] fixup! quic: minor cleanups in quic_buffer --- src/quic/node_quic_buffer.cc | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/src/quic/node_quic_buffer.cc b/src/quic/node_quic_buffer.cc index d3762589c84127..acbe4e34c3712c 100644 --- a/src/quic/node_quic_buffer.cc +++ b/src/quic/node_quic_buffer.cc @@ -17,10 +17,11 @@ void QuicBufferChunk::MemoryInfo(MemoryTracker* tracker) const { size_t QuicBuffer::Push(uv_buf_t* bufs, size_t nbufs, DoneCB done) { size_t len = 0; - if (UNLIKELY(nbufs == 0 || bufs == nullptr || is_empty(bufs[0]))) { + if (UNLIKELY(nbufs == 0)) { done(0); return 0; } + DCHECK_NOT_NULL(bufs); size_t n = 0; while (nbufs > 1) { if (!is_empty(bufs[n])) { @@ -30,8 +31,14 @@ size_t QuicBuffer::Push(uv_buf_t* bufs, size_t nbufs, DoneCB done) { n++; nbufs--; } - len += bufs[n].len; - Push(bufs[n], done); + if (!is_empty(bufs[n])) { + Push(bufs[n], done); + len += bufs[n].len; + } + // Special case if all the bufs were empty. + if (UNLIKELY(len == 0)) + done(0); + return len; }