From 6e47a4b4d2237c299f13d195afc0903d4cc83469 Mon Sep 17 00:00:00 2001 From: Denys Otrishko Date: Sun, 10 Nov 2019 14:22:46 +0200 Subject: [PATCH 1/2] http2: small clean up in OnStreamRead --- src/node_http2.cc | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/src/node_http2.cc b/src/node_http2.cc index 9421c36f3be561..2bcbcbe0784747 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1867,14 +1867,10 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) { // call in OnStreamAfterWrite() immediately provides data. If that does // happen, we concatenate the data we received with the already-stored // pending input data, slicing off the already processed part. - AllocatedBuffer new_buf = env()->AllocateManaged( - stream_buf_.len - stream_buf_offset_ + nread); - memcpy(new_buf.data(), - stream_buf_.base + stream_buf_offset_, - stream_buf_.len - stream_buf_offset_); - memcpy(new_buf.data() + stream_buf_.len - stream_buf_offset_, - buf.data(), - nread); + size_t pending_len = stream_buf_.len - stream_buf_offset_; + AllocatedBuffer new_buf = env()->AllocateManaged(pending_len + nread); + memcpy(new_buf.data(), stream_buf_.base + stream_buf_offset_, pending_len); + memcpy(new_buf.data() + pending_len, buf.data(), nread); buf = std::move(new_buf); nread = buf.size(); stream_buf_offset_ = 0; From 542924ebf46172abf023f2484713fd0f44bb53bd Mon Sep 17 00:00:00 2001 From: Denys Otrishko Date: Sun, 1 Dec 2019 18:20:01 +0200 Subject: [PATCH 2/2] http2: streamline OnStreamRead streamline memory accounting * avoid consecutive decrement/increment session memory calls. * only Resize the buffer when it is needed. * flip `stream_buf_offset_` condition to the LIKELY case. --- src/node_http2.cc | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/node_http2.cc b/src/node_http2.cc index 2bcbcbe0784747..33960af1845a15 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -1862,7 +1862,11 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) { statistics_.data_received += nread; - if (UNLIKELY(stream_buf_offset_ > 0)) { + if (LIKELY(stream_buf_offset_ == 0)) { + // Shrink to the actual amount of used data. + buf.Resize(nread); + IncrementCurrentSessionMemory(nread); + } else { // This is a very unlikely case, and should only happen if the ReadStart() // call in OnStreamAfterWrite() immediately provides data. If that does // happen, we concatenate the data we received with the already-stored @@ -1871,20 +1875,18 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) { AllocatedBuffer new_buf = env()->AllocateManaged(pending_len + nread); memcpy(new_buf.data(), stream_buf_.base + stream_buf_offset_, pending_len); memcpy(new_buf.data() + pending_len, buf.data(), nread); + + // The data in stream_buf_ is already accounted for, add nread received + // bytes to session memory but remove the already processed + // stream_buf_offset_ bytes. + IncrementCurrentSessionMemory(nread - stream_buf_offset_); + buf = std::move(new_buf); nread = buf.size(); stream_buf_offset_ = 0; stream_buf_ab_.Reset(); - - // We have now fully processed the stream_buf_ input chunk (by moving the - // remaining part into buf, which will be accounted for below). - DecrementCurrentSessionMemory(stream_buf_.len); } - // Shrink to the actual amount of used data. - buf.Resize(nread); - IncrementCurrentSessionMemory(nread); - // Remember the current buffer, so that OnDataChunkReceived knows the // offset of a DATA frame's data into the socket read buffer. stream_buf_ = uv_buf_init(buf.data(), nread);