From 8a0d101adfb7c1618fc26d03cf6e63340c2dc18a Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Thu, 10 Aug 2017 16:54:42 +0200 Subject: [PATCH] http2: use per-environment buffers As discussed in the review for https://github.com/nodejs/node/pull/14239, these buffers should be per-Environment rather than static. PR-URL: https://github.com/nodejs/node/pull/14744 Reviewed-By: James M Snell --- lib/internal/http2/core.js | 2 +- lib/internal/http2/util.js | 6 +-- src/env-inl.h | 10 ++++ src/env.h | 8 ++++ src/node_http2.cc | 97 +++++++++++++++----------------------- 5 files changed, 60 insertions(+), 63 deletions(-) diff --git a/lib/internal/http2/core.js b/lib/internal/http2/core.js index da8edb18cfdef7..972cdc97333739 100755 --- a/lib/internal/http2/core.js +++ b/lib/internal/http2/core.js @@ -73,7 +73,7 @@ const kType = Symbol('type'); const kDefaultSocketTimeout = 2 * 60 * 1000; const kRenegTest = /TLS session renegotiation disabled for this socket/; -const paddingBuffer = new Uint32Array(binding.paddingArrayBuffer); +const { paddingBuffer } = binding; const { NGHTTP2_CANCEL, diff --git a/lib/internal/http2/util.js b/lib/internal/http2/util.js index 5054d26e593aa3..09f55fdc65e309 100644 --- a/lib/internal/http2/util.js +++ b/lib/internal/http2/util.js @@ -114,16 +114,14 @@ const kNoPayloadMethods = new Set([ // the native side with values that are filled in on demand, the js code then // reads those values out. The set of IDX constants that follow identify the // relevant data positions within these buffers. -const settingsBuffer = new Uint32Array(binding.settingsArrayBuffer); -const optionsBuffer = new Uint32Array(binding.optionsArrayBuffer); +const { settingsBuffer, optionsBuffer } = binding; // Note that Float64Array is used here because there is no Int64Array available // and these deal with numbers that can be beyond the range of Uint32 and Int32. // The values set on the native side will always be integers. This is not a // unique example of this, this pattern can be found in use in other parts of // Node.js core as a performance optimization. -const sessionState = new Float64Array(binding.sessionStateArrayBuffer); -const streamState = new Float64Array(binding.streamStateArrayBuffer); +const { sessionState, streamState } = binding; const IDX_SETTINGS_HEADER_TABLE_SIZE = 0; const IDX_SETTINGS_ENABLE_PUSH = 1; diff --git a/src/env-inl.h b/src/env-inl.h index b248fd8a3e7e60..888dd807c11e15 100644 --- a/src/env-inl.h +++ b/src/env-inl.h @@ -329,6 +329,7 @@ inline Environment::~Environment() { delete[] heap_statistics_buffer_; delete[] heap_space_statistics_buffer_; delete[] http_parser_buffer_; + free(http2_state_buffer_); } inline v8::Isolate* Environment::isolate() const { @@ -478,6 +479,15 @@ inline void Environment::set_http_parser_buffer(char* buffer) { http_parser_buffer_ = buffer; } +inline http2::http2_state* Environment::http2_state_buffer() const { + return http2_state_buffer_; +} + +inline void Environment::set_http2_state_buffer(http2::http2_state* buffer) { + CHECK_EQ(http2_state_buffer_, nullptr); // Should be set only once. + http2_state_buffer_ = buffer; +} + inline double* Environment::fs_stats_field_array() const { return fs_stats_field_array_; } diff --git a/src/env.h b/src/env.h index f0774e61a65d08..55340947fdd565 100644 --- a/src/env.h +++ b/src/env.h @@ -43,6 +43,10 @@ namespace node { +namespace http2 { +struct http2_state; +} + // Pick an index that's hopefully out of the way when we're embedded inside // another application. Performance-wise or memory-wise it doesn't matter: // Context::SetAlignedPointerInEmbedderData() is backed by a FixedArray, @@ -599,6 +603,9 @@ class Environment { inline char* http_parser_buffer() const; inline void set_http_parser_buffer(char* buffer); + inline http2::http2_state* http2_state_buffer() const; + inline void set_http2_state_buffer(http2::http2_state* buffer); + inline double* fs_stats_field_array() const; inline void set_fs_stats_field_array(double* fields); @@ -705,6 +712,7 @@ class Environment { double* heap_space_statistics_buffer_ = nullptr; char* http_parser_buffer_; + http2::http2_state* http2_state_buffer_ = nullptr; double* fs_stats_field_array_; diff --git a/src/node_http2.cc b/src/node_http2.cc index 28661f9a668ba9..d5dbdb80a0431d 100755 --- a/src/node_http2.cc +++ b/src/node_http2.cc @@ -7,10 +7,12 @@ namespace node { using v8::ArrayBuffer; using v8::Boolean; using v8::Context; +using v8::Float64Array; using v8::Function; using v8::Integer; using v8::String; using v8::Uint32; +using v8::Uint32Array; using v8::Undefined; namespace http2 { @@ -57,27 +59,18 @@ enum Http2OptionsIndex { IDX_OPTIONS_FLAGS }; -static uint32_t http2_padding_buffer[3]; -static uint32_t http2_options_buffer[IDX_OPTIONS_FLAGS + 1]; -static uint32_t http2_settings_buffer[IDX_SETTINGS_COUNT + 1]; -static double http2_session_state_buffer[IDX_SESSION_STATE_COUNT]; -static double http2_stream_state_buffer[IDX_STREAM_STATE_COUNT]; - -static const size_t http2_options_buffer_byte_length = - sizeof(http2_options_buffer) * (IDX_OPTIONS_FLAGS + 1); -static const size_t http2_settings_buffer_byte_length = - sizeof(http2_settings_buffer) * (IDX_SETTINGS_COUNT + 1); -static const size_t http2_padding_buffer_byte_length = - sizeof(http2_padding_buffer) * 3; -static const size_t http2_stream_state_buffer_byte_length = - sizeof(http2_stream_state_buffer) * IDX_STREAM_STATE_COUNT; -static const size_t http2_session_state_buffer_byte_length = - sizeof(http2_session_state_buffer) * IDX_SESSION_STATE_COUNT; +struct http2_state { + uint32_t padding_buffer[3]; + uint32_t options_buffer[IDX_OPTIONS_FLAGS + 1]; + uint32_t settings_buffer[IDX_SETTINGS_COUNT + 1]; + double session_state_buffer[IDX_SESSION_STATE_COUNT]; + double stream_state_buffer[IDX_STREAM_STATE_COUNT]; +}; Http2Options::Http2Options(Environment* env) { nghttp2_option_new(&options_); - uint32_t* buffer = http2_options_buffer; + uint32_t* buffer = env->http2_state_buffer()->options_buffer; uint32_t flags = buffer[IDX_OPTIONS_FLAGS]; if (flags & (1 << IDX_OPTIONS_MAX_DEFLATE_DYNAMIC_TABLE_SIZE)) { @@ -126,7 +119,7 @@ ssize_t Http2Session::OnCallbackPadding(size_t frameLen, Context::Scope context_scope(context); if (object()->Has(context, env()->ongetpadding_string()).FromJust()) { - uint32_t* buffer = http2_padding_buffer; + uint32_t* buffer = env()->http2_state_buffer()->padding_buffer; buffer[0] = frameLen; buffer[1] = maxPayloadLen; MakeCallback(env()->ongetpadding_string(), 0, nullptr); @@ -167,7 +160,7 @@ void PackSettings(const FunctionCallbackInfo& args) { std::vector entries; entries.reserve(6); - uint32_t* buffer = http2_settings_buffer; + uint32_t* buffer = env->http2_state_buffer()->settings_buffer; uint32_t flags = buffer[IDX_SETTINGS_COUNT]; if (flags & (1 << IDX_SETTINGS_HEADER_TABLE_SIZE)) { @@ -226,7 +219,8 @@ void PackSettings(const FunctionCallbackInfo& args) { // Used to fill in the spec defined initial values for each setting. void RefreshDefaultSettings(const FunctionCallbackInfo& args) { DEBUG_HTTP2("Http2Session: refreshing default settings\n"); - uint32_t* buffer = http2_settings_buffer; + Environment* env = Environment::GetCurrent(args); + uint32_t* buffer = env->http2_state_buffer()->settings_buffer; buffer[IDX_SETTINGS_HEADER_TABLE_SIZE] = DEFAULT_SETTINGS_HEADER_TABLE_SIZE; buffer[IDX_SETTINGS_ENABLE_PUSH] = @@ -245,13 +239,14 @@ void RefreshDefaultSettings(const FunctionCallbackInfo& args) { template void RefreshSettings(const FunctionCallbackInfo& args) { DEBUG_HTTP2("Http2Session: refreshing settings for session\n"); + Environment* env = Environment::GetCurrent(args); CHECK_EQ(args.Length(), 1); CHECK(args[0]->IsObject()); Http2Session* session; ASSIGN_OR_RETURN_UNWRAP(&session, args[0].As()); nghttp2_session* s = session->session(); - uint32_t* buffer = http2_settings_buffer; + uint32_t* buffer = env->http2_state_buffer()->settings_buffer; buffer[IDX_SETTINGS_HEADER_TABLE_SIZE] = fn(s, NGHTTP2_SETTINGS_HEADER_TABLE_SIZE); buffer[IDX_SETTINGS_MAX_CONCURRENT_STREAMS] = @@ -269,9 +264,10 @@ void RefreshSettings(const FunctionCallbackInfo& args) { // Used to fill in the spec defined initial values for each setting. void RefreshSessionState(const FunctionCallbackInfo& args) { DEBUG_HTTP2("Http2Session: refreshing session state\n"); + Environment* env = Environment::GetCurrent(args); CHECK_EQ(args.Length(), 1); CHECK(args[0]->IsObject()); - double* buffer = http2_session_state_buffer; + double* buffer = env->http2_state_buffer()->session_state_buffer; Http2Session* session; ASSIGN_OR_RETURN_UNWRAP(&session, args[0].As()); nghttp2_session* s = session->session(); @@ -308,7 +304,7 @@ void RefreshStreamState(const FunctionCallbackInfo& args) { nghttp2_session* s = session->session(); Nghttp2Stream* stream; - double* buffer = http2_stream_state_buffer; + double* buffer = env->http2_state_buffer()->stream_state_buffer; if ((stream = session->FindStream(id)) == nullptr) { buffer[IDX_STREAM_STATE] = NGHTTP2_STREAM_STATE_IDLE; @@ -418,8 +414,9 @@ void Http2Session::SubmitPriority(const FunctionCallbackInfo& args) { void Http2Session::SubmitSettings(const FunctionCallbackInfo& args) { Http2Session* session; ASSIGN_OR_RETURN_UNWRAP(&session, args.Holder()); + Environment* env = session->env(); - uint32_t* buffer = http2_settings_buffer; + uint32_t* buffer = env->http2_state_buffer()->settings_buffer; uint32_t flags = buffer[IDX_SETTINGS_COUNT]; std::vector entries; @@ -1148,43 +1145,27 @@ void Initialize(Local target, Isolate* isolate = env->isolate(); HandleScope scope(isolate); - // Initialize the buffer used for padding callbacks - target->Set(context, - FIXED_ONE_BYTE_STRING(isolate, "paddingArrayBuffer"), - ArrayBuffer::New(isolate, - &http2_padding_buffer, - http2_padding_buffer_byte_length)) - .FromJust(); + http2_state* state = Calloc(1); + env->set_http2_state_buffer(state); + auto state_ab = ArrayBuffer::New(isolate, state, sizeof(*state)); - // Initialize the buffer used to store the session state - target->Set(context, - FIXED_ONE_BYTE_STRING(isolate, "sessionStateArrayBuffer"), - ArrayBuffer::New(isolate, - &http2_session_state_buffer, - http2_session_state_buffer_byte_length)) - .FromJust(); +#define SET_STATE_TYPEDARRAY(name, type, field) \ + target->Set(context, \ + FIXED_ONE_BYTE_STRING(isolate, (name)), \ + type::New(state_ab, \ + offsetof(http2_state, field), \ + arraysize(state->field))) \ + .FromJust() + // Initialize the buffer used for padding callbacks + SET_STATE_TYPEDARRAY("paddingBuffer", Uint32Array, padding_buffer); + // Initialize the buffer used to store the session state + SET_STATE_TYPEDARRAY("sessionState", Float64Array, session_state_buffer); // Initialize the buffer used to store the stream state - target->Set(context, - FIXED_ONE_BYTE_STRING(isolate, "streamStateArrayBuffer"), - ArrayBuffer::New(isolate, - &http2_stream_state_buffer, - http2_stream_state_buffer_byte_length)) - .FromJust(); - - target->Set(context, - FIXED_ONE_BYTE_STRING(isolate, "settingsArrayBuffer"), - ArrayBuffer::New(isolate, - &http2_settings_buffer, - http2_settings_buffer_byte_length)) - .FromJust(); - - target->Set(context, - FIXED_ONE_BYTE_STRING(isolate, "optionsArrayBuffer"), - ArrayBuffer::New(isolate, - &http2_options_buffer, - http2_options_buffer_byte_length)) - .FromJust(); + SET_STATE_TYPEDARRAY("streamState", Float64Array, stream_state_buffer); + SET_STATE_TYPEDARRAY("settingsBuffer", Uint32Array, settings_buffer); + SET_STATE_TYPEDARRAY("optionsBuffer", Uint32Array, options_buffer); +#undef SET_STATE_TYPEDARRAY // Method to fetch the nghttp2 string description of an nghttp2 error code env->SetMethod(target, "nghttp2ErrorString", HttpErrorString);