Skip to content

Commit e9388c8

Browse files
RaisinTentargos
authored andcommitted
src: remove usage of AllocatedBuffer from node_http2
Signed-off-by: Darshan Sen <[email protected]> PR-URL: #40584 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Anna Henningsen <[email protected]>
1 parent 179a5c5 commit e9388c8

File tree

2 files changed

+60
-39
lines changed

2 files changed

+60
-39
lines changed

src/node_http2.cc

+57-35
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,11 @@ namespace node {
2121
using v8::Array;
2222
using v8::ArrayBuffer;
2323
using v8::ArrayBufferView;
24+
using v8::BackingStore;
2425
using v8::Boolean;
2526
using v8::Context;
2627
using v8::EscapableHandleScope;
28+
using v8::False;
2729
using v8::Function;
2830
using v8::FunctionCallbackInfo;
2931
using v8::FunctionTemplate;
@@ -37,6 +39,7 @@ using v8::Number;
3739
using v8::Object;
3840
using v8::ObjectTemplate;
3941
using v8::String;
42+
using v8::True;
4043
using v8::Uint8Array;
4144
using v8::Undefined;
4245
using v8::Value;
@@ -267,17 +270,20 @@ Local<Value> Http2Settings::Pack(
267270
size_t count,
268271
const nghttp2_settings_entry* entries) {
269272
EscapableHandleScope scope(env->isolate());
270-
const size_t size = count * 6;
271-
AllocatedBuffer buffer = AllocatedBuffer::AllocateManaged(env, size);
272-
ssize_t ret =
273-
nghttp2_pack_settings_payload(
274-
reinterpret_cast<uint8_t*>(buffer.data()),
275-
size,
276-
entries,
277-
count);
278-
Local<Value> buf = Undefined(env->isolate());
279-
if (ret >= 0) buf = buffer.ToBuffer().ToLocalChecked();
280-
return scope.Escape(buf);
273+
std::unique_ptr<BackingStore> bs;
274+
{
275+
NoArrayBufferZeroFillScope no_zero_fill_scope(env->isolate_data());
276+
bs = ArrayBuffer::NewBackingStore(env->isolate(), count * 6);
277+
}
278+
if (nghttp2_pack_settings_payload(static_cast<uint8_t*>(bs->Data()),
279+
bs->ByteLength(),
280+
entries,
281+
count) < 0) {
282+
return scope.Escape(Undefined(env->isolate()));
283+
}
284+
Local<ArrayBuffer> ab = ArrayBuffer::New(env->isolate(), std::move(bs));
285+
return scope.Escape(Buffer::New(env, ab, 0, ab->ByteLength())
286+
.FromMaybe(Local<Value>()));
281287
}
282288

283289
// Updates the shared TypedArray with the current remote or local settings for
@@ -323,7 +329,7 @@ void Http2Settings::Done(bool ack) {
323329
double duration = (end - startTime_) / 1e6;
324330

325331
Local<Value> argv[] = {
326-
ack ? v8::True(env()->isolate()) : v8::False(env()->isolate()),
332+
ack ? True(env()->isolate()) : False(env()->isolate()),
327333
Number::New(env()->isolate(), duration)
328334
};
329335
MakeCallback(callback(), arraysize(argv), argv);
@@ -368,19 +374,23 @@ Origins::Origins(
368374
return;
369375
}
370376

371-
buf_ = AllocatedBuffer::AllocateManaged(
372-
env,
373-
(alignof(nghttp2_origin_entry) - 1) +
374-
count_ * sizeof(nghttp2_origin_entry) +
375-
origin_string_len);
377+
{
378+
NoArrayBufferZeroFillScope no_zero_fill_scope(env->isolate_data());
379+
bs_ = ArrayBuffer::NewBackingStore(env->isolate(),
380+
alignof(nghttp2_origin_entry) - 1 +
381+
count_ * sizeof(nghttp2_origin_entry) +
382+
origin_string_len);
383+
}
376384

377385
// Make sure the start address is aligned appropriately for an nghttp2_nv*.
378-
char* start = AlignUp(buf_.data(), alignof(nghttp2_origin_entry));
386+
char* start = AlignUp(static_cast<char*>(bs_->Data()),
387+
alignof(nghttp2_origin_entry));
379388
char* origin_contents = start + (count_ * sizeof(nghttp2_origin_entry));
380389
nghttp2_origin_entry* const nva =
381390
reinterpret_cast<nghttp2_origin_entry*>(start);
382391

383-
CHECK_LE(origin_contents + origin_string_len, buf_.data() + buf_.size());
392+
CHECK_LE(origin_contents + origin_string_len,
393+
static_cast<char*>(bs_->Data()) + bs_->ByteLength());
384394
CHECK_EQ(origin_string->WriteOneByte(
385395
env->isolate(),
386396
reinterpret_cast<uint8_t*>(origin_contents),
@@ -819,7 +829,7 @@ void Http2Session::ConsumeHTTP2Data() {
819829
DecrementCurrentSessionMemory(stream_buf_.len);
820830
stream_buf_offset_ = 0;
821831
stream_buf_ab_.Reset();
822-
stream_buf_allocation_.clear();
832+
stream_buf_allocation_.reset();
823833
stream_buf_ = uv_buf_init(nullptr, 0);
824834

825835
// 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) {
12471257

12481258
Local<ArrayBuffer> ab;
12491259
if (session->stream_buf_ab_.IsEmpty()) {
1250-
ab = session->stream_buf_allocation_.ToArrayBuffer();
1260+
ab = ArrayBuffer::New(env->isolate(),
1261+
std::move(session->stream_buf_allocation_));
12511262
session->stream_buf_ab_.Reset(env->isolate(), ab);
12521263
} else {
12531264
ab = PersistentToLocal::Strong(session->stream_buf_ab_);
@@ -1823,7 +1834,7 @@ Http2Stream* Http2Session::SubmitRequest(
18231834
}
18241835

18251836
uv_buf_t Http2Session::OnStreamAlloc(size_t suggested_size) {
1826-
return AllocatedBuffer::AllocateManaged(env(), suggested_size).release();
1837+
return env()->allocate_managed_buffer(suggested_size);
18271838
}
18281839

18291840
// 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_) {
18331844
Http2Scope h2scope(this);
18341845
CHECK_NOT_NULL(stream_);
18351846
Debug(this, "receiving %d bytes, offset %d", nread, stream_buf_offset_);
1836-
AllocatedBuffer buf(env(), buf_);
1847+
std::unique_ptr<BackingStore> bs = env()->release_managed_buffer(buf_);
18371848

18381849
// Only pass data on if nread > 0
18391850
if (nread <= 0) {
@@ -1843,24 +1854,34 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) {
18431854
return;
18441855
}
18451856

1857+
CHECK_LE(static_cast<size_t>(nread), bs->ByteLength());
1858+
18461859
statistics_.data_received += nread;
18471860

18481861
if (LIKELY(stream_buf_offset_ == 0)) {
18491862
// Shrink to the actual amount of used data.
1850-
buf.Resize(nread);
1863+
bs = BackingStore::Reallocate(env()->isolate(), std::move(bs), nread);
18511864
} else {
18521865
// This is a very unlikely case, and should only happen if the ReadStart()
18531866
// call in OnStreamAfterWrite() immediately provides data. If that does
18541867
// happen, we concatenate the data we received with the already-stored
18551868
// pending input data, slicing off the already processed part.
18561869
size_t pending_len = stream_buf_.len - stream_buf_offset_;
1857-
AllocatedBuffer new_buf =
1858-
AllocatedBuffer::AllocateManaged(env(), pending_len + nread);
1859-
memcpy(new_buf.data(), stream_buf_.base + stream_buf_offset_, pending_len);
1860-
memcpy(new_buf.data() + pending_len, buf.data(), nread);
1861-
1862-
buf = std::move(new_buf);
1863-
nread = buf.size();
1870+
std::unique_ptr<BackingStore> new_bs;
1871+
{
1872+
NoArrayBufferZeroFillScope no_zero_fill_scope(env()->isolate_data());
1873+
new_bs = ArrayBuffer::NewBackingStore(env()->isolate(),
1874+
pending_len + nread);
1875+
}
1876+
memcpy(static_cast<char*>(new_bs->Data()),
1877+
stream_buf_.base + stream_buf_offset_,
1878+
pending_len);
1879+
memcpy(static_cast<char*>(new_bs->Data()) + pending_len,
1880+
bs->Data(),
1881+
nread);
1882+
1883+
bs = std::move(new_bs);
1884+
nread = bs->ByteLength();
18641885
stream_buf_offset_ = 0;
18651886
stream_buf_ab_.Reset();
18661887

@@ -1873,12 +1894,13 @@ void Http2Session::OnStreamRead(ssize_t nread, const uv_buf_t& buf_) {
18731894

18741895
// Remember the current buffer, so that OnDataChunkReceived knows the
18751896
// offset of a DATA frame's data into the socket read buffer.
1876-
stream_buf_ = uv_buf_init(buf.data(), static_cast<unsigned int>(nread));
1897+
stream_buf_ = uv_buf_init(static_cast<char*>(bs->Data()),
1898+
static_cast<unsigned int>(nread));
18771899

18781900
// Store this so we can create an ArrayBuffer for read data from it.
18791901
// DATA frames will be emitted as slices of that ArrayBuffer to avoid having
18801902
// to copy memory.
1881-
stream_buf_allocation_ = std::move(buf);
1903+
stream_buf_allocation_ = std::move(bs);
18821904

18831905
ConsumeHTTP2Data();
18841906

@@ -2023,7 +2045,7 @@ void Http2Stream::Close(int32_t code) {
20232045
Debug(this, "closed with code %d", code);
20242046
}
20252047

2026-
ShutdownWrap* Http2Stream::CreateShutdownWrap(v8::Local<v8::Object> object) {
2048+
ShutdownWrap* Http2Stream::CreateShutdownWrap(Local<Object> object) {
20272049
// DoShutdown() always finishes synchronously, so there's no need to create
20282050
// a structure to store asynchronous context.
20292051
return nullptr;
@@ -3049,7 +3071,7 @@ void Http2Ping::Done(bool ack, const uint8_t* payload) {
30493071
}
30503072

30513073
Local<Value> argv[] = {
3052-
ack ? v8::True(isolate) : v8::False(isolate),
3074+
ack ? True(isolate) : False(isolate),
30533075
Number::New(isolate, duration_ms),
30543076
buf
30553077
};

src/node_http2.h

+3-4
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
#include "nghttp2/nghttp2.h"
99

1010
#include "env.h"
11-
#include "allocated_buffer.h"
1211
#include "aliased_struct.h"
1312
#include "node_http2_state.h"
1413
#include "node_http_common.h"
@@ -897,7 +896,7 @@ class Http2Session : public AsyncWrap,
897896
// When processing input data, either stream_buf_ab_ or stream_buf_allocation_
898897
// will be set. stream_buf_ab_ is lazily created from stream_buf_allocation_.
899898
v8::Global<v8::ArrayBuffer> stream_buf_ab_;
900-
AllocatedBuffer stream_buf_allocation_;
899+
std::unique_ptr<v8::BackingStore> stream_buf_allocation_;
901900
size_t stream_buf_offset_ = 0;
902901
// Custom error code for errors that originated inside one of the callbacks
903902
// called by nghttp2_session_mem_recv.
@@ -1040,7 +1039,7 @@ class Origins {
10401039
~Origins() = default;
10411040

10421041
const nghttp2_origin_entry* operator*() const {
1043-
return reinterpret_cast<const nghttp2_origin_entry*>(buf_.data());
1042+
return static_cast<const nghttp2_origin_entry*>(bs_->Data());
10441043
}
10451044

10461045
size_t length() const {
@@ -1049,7 +1048,7 @@ class Origins {
10491048

10501049
private:
10511050
size_t count_;
1052-
AllocatedBuffer buf_;
1051+
std::unique_ptr<v8::BackingStore> bs_;
10531052
};
10541053

10551054
#define HTTP2_HIDDEN_CONSTANTS(V) \

0 commit comments

Comments
 (0)