From 2c8a6ec28e12577150d344305b4fd5e17410c45d Mon Sep 17 00:00:00 2001 From: Darshan Sen Date: Sun, 24 Oct 2021 14:20:15 +0530 Subject: [PATCH] src: remove usage of `AllocatedBuffer` from `node_http2` Signed-off-by: Darshan Sen PR-URL: https://github.com/nodejs/node/pull/40584 Reviewed-By: James M Snell Reviewed-By: Anna Henningsen --- src/node_http2.cc | 92 +++++++++++++++++++++++++++++------------------ src/node_http2.h | 7 ++-- 2 files changed, 60 insertions(+), 39 deletions(-) diff --git a/src/node_http2.cc b/src/node_http2.cc index c01fa4d5255077..f5a1db0d022a18 100644 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -21,9 +21,11 @@ namespace node { using v8::Array; using v8::ArrayBuffer; using v8::ArrayBufferView; +using v8::BackingStore; using v8::Boolean; using v8::Context; using v8::EscapableHandleScope; +using v8::False; using v8::Function; using v8::FunctionCallbackInfo; using v8::FunctionTemplate; @@ -37,6 +39,7 @@ using v8::Number; using v8::Object; using v8::ObjectTemplate; using v8::String; +using v8::True; using v8::Uint8Array; using v8::Undefined; using v8::Value; @@ -267,17 +270,20 @@ Local Http2Settings::Pack( size_t count, const nghttp2_settings_entry* entries) { EscapableHandleScope scope(env->isolate()); - const size_t size = count * 6; - AllocatedBuffer buffer = AllocatedBuffer::AllocateManaged(env, size); - ssize_t ret = - nghttp2_pack_settings_payload( - reinterpret_cast(buffer.data()), - size, - entries, - count); - Local buf = Undefined(env->isolate()); - if (ret >= 0) buf = buffer.ToBuffer().ToLocalChecked(); - return scope.Escape(buf); + std::unique_ptr bs; + { + NoArrayBufferZeroFillScope no_zero_fill_scope(env->isolate_data()); + bs = ArrayBuffer::NewBackingStore(env->isolate(), count * 6); + } + if (nghttp2_pack_settings_payload(static_cast(bs->Data()), + bs->ByteLength(), + entries, + count) < 0) { + return scope.Escape(Undefined(env->isolate())); + } + Local ab = ArrayBuffer::New(env->isolate(), std::move(bs)); + return scope.Escape(Buffer::New(env, ab, 0, ab->ByteLength()) + .FromMaybe(Local())); } // Updates the shared TypedArray with the current remote or local settings for @@ -323,7 +329,7 @@ void Http2Settings::Done(bool ack) { double duration = (end - startTime_) / 1e6; Local argv[] = { - ack ? v8::True(env()->isolate()) : v8::False(env()->isolate()), + ack ? True(env()->isolate()) : False(env()->isolate()), Number::New(env()->isolate(), duration) }; MakeCallback(callback(), arraysize(argv), argv); @@ -368,19 +374,23 @@ Origins::Origins( return; } - buf_ = AllocatedBuffer::AllocateManaged( - env, - (alignof(nghttp2_origin_entry) - 1) + - count_ * sizeof(nghttp2_origin_entry) + - origin_string_len); + { + NoArrayBufferZeroFillScope no_zero_fill_scope(env->isolate_data()); + bs_ = ArrayBuffer::NewBackingStore(env->isolate(), + alignof(nghttp2_origin_entry) - 1 + + count_ * sizeof(nghttp2_origin_entry) + + origin_string_len); + } // Make sure the start address is aligned appropriately for an nghttp2_nv*. - char* start = AlignUp(buf_.data(), alignof(nghttp2_origin_entry)); + char* start = AlignUp(static_cast(bs_->Data()), + alignof(nghttp2_origin_entry)); char* origin_contents = start + (count_ * sizeof(nghttp2_origin_entry)); nghttp2_origin_entry* const nva = reinterpret_cast(start); - CHECK_LE(origin_contents + origin_string_len, buf_.data() + buf_.size()); + CHECK_LE(origin_contents + origin_string_len, + static_cast(bs_->Data()) + bs_->ByteLength()); CHECK_EQ(origin_string->WriteOneByte( env->isolate(), reinterpret_cast(origin_contents), @@ -819,7 +829,7 @@ void Http2Session::ConsumeHTTP2Data() { DecrementCurrentSessionMemory(stream_buf_.len); stream_buf_offset_ = 0; stream_buf_ab_.Reset(); - stream_buf_allocation_.clear(); + stream_buf_allocation_.reset(); stream_buf_ = uv_buf_init(nullptr, 0); // Send any data that was queued up while processing the received data. @@ -1247,7 +1257,8 @@ void Http2StreamListener::OnStreamRead(ssize_t nread, const uv_buf_t& buf) { Local ab; if (session->stream_buf_ab_.IsEmpty()) { - ab = session->stream_buf_allocation_.ToArrayBuffer(); + ab = ArrayBuffer::New(env->isolate(), + std::move(session->stream_buf_allocation_)); session->stream_buf_ab_.Reset(env->isolate(), ab); } else { ab = PersistentToLocal::Strong(session->stream_buf_ab_); @@ -1823,7 +1834,7 @@ Http2Stream* Http2Session::SubmitRequest( } uv_buf_t Http2Session::OnStreamAlloc(size_t suggested_size) { - return AllocatedBuffer::AllocateManaged(env(), suggested_size).release(); + return env()->allocate_managed_buffer(suggested_size); } // Callback used to receive inbound data from the i/o stream @@ -1833,7 +1844,7 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) { Http2Scope h2scope(this); CHECK_NOT_NULL(stream_); Debug(this, "receiving %d bytes, offset %d", nread, stream_buf_offset_); - AllocatedBuffer buf(env(), buf_); + std::unique_ptr bs = env()->release_managed_buffer(buf_); // Only pass data on if nread > 0 if (nread <= 0) { @@ -1843,24 +1854,34 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) { return; } + CHECK_LE(static_cast(nread), bs->ByteLength()); + statistics_.data_received += nread; if (LIKELY(stream_buf_offset_ == 0)) { // Shrink to the actual amount of used data. - buf.Resize(nread); + bs = BackingStore::Reallocate(env()->isolate(), std::move(bs), 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 // pending input data, slicing off the already processed part. size_t pending_len = stream_buf_.len - stream_buf_offset_; - AllocatedBuffer new_buf = - AllocatedBuffer::AllocateManaged(env(), 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(); + std::unique_ptr new_bs; + { + NoArrayBufferZeroFillScope no_zero_fill_scope(env()->isolate_data()); + new_bs = ArrayBuffer::NewBackingStore(env()->isolate(), + pending_len + nread); + } + memcpy(static_cast(new_bs->Data()), + stream_buf_.base + stream_buf_offset_, + pending_len); + memcpy(static_cast(new_bs->Data()) + pending_len, + bs->Data(), + nread); + + bs = std::move(new_bs); + nread = bs->ByteLength(); stream_buf_offset_ = 0; stream_buf_ab_.Reset(); @@ -1873,12 +1894,13 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) { // 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(), static_cast(nread)); + stream_buf_ = uv_buf_init(static_cast(bs->Data()), + static_cast(nread)); // Store this so we can create an ArrayBuffer for read data from it. // DATA frames will be emitted as slices of that ArrayBuffer to avoid having // to copy memory. - stream_buf_allocation_ = std::move(buf); + stream_buf_allocation_ = std::move(bs); ConsumeHTTP2Data(); @@ -2023,7 +2045,7 @@ void Http2Stream::Close(int32_t code) { Debug(this, "closed with code %d", code); } -ShutdownWrap* Http2Stream::CreateShutdownWrap(v8::Local object) { +ShutdownWrap* Http2Stream::CreateShutdownWrap(Local object) { // DoShutdown() always finishes synchronously, so there's no need to create // a structure to store asynchronous context. return nullptr; @@ -3049,7 +3071,7 @@ void Http2Ping::Done(bool ack, const uint8_t* payload) { } Local argv[] = { - ack ? v8::True(isolate) : v8::False(isolate), + ack ? True(isolate) : False(isolate), Number::New(isolate, duration_ms), buf }; diff --git a/src/node_http2.h b/src/node_http2.h index 4d267e647d3494..e3d4d70b2d1fd5 100644 --- a/src/node_http2.h +++ b/src/node_http2.h @@ -8,7 +8,6 @@ #include "nghttp2/nghttp2.h" #include "env.h" -#include "allocated_buffer.h" #include "aliased_struct.h" #include "node_http2_state.h" #include "node_http_common.h" @@ -897,7 +896,7 @@ class Http2Session : public AsyncWrap, // When processing input data, either stream_buf_ab_ or stream_buf_allocation_ // will be set. stream_buf_ab_ is lazily created from stream_buf_allocation_. v8::Global stream_buf_ab_; - AllocatedBuffer stream_buf_allocation_; + std::unique_ptr stream_buf_allocation_; size_t stream_buf_offset_ = 0; // Custom error code for errors that originated inside one of the callbacks // called by nghttp2_session_mem_recv. @@ -1040,7 +1039,7 @@ class Origins { ~Origins() = default; const nghttp2_origin_entry* operator*() const { - return reinterpret_cast(buf_.data()); + return static_cast(bs_->Data()); } size_t length() const { @@ -1049,7 +1048,7 @@ class Origins { private: size_t count_; - AllocatedBuffer buf_; + std::unique_ptr bs_; }; #define HTTP2_HIDDEN_CONSTANTS(V) \