http2: use per-environment buffers

As discussed in the review for
#14239, these buffers should
be per-Environment rather than static.

PR-URL: #14744
Reviewed-By: James M Snell <[email protected]>

Author: addaleax

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,

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;

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_;
 }

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_;
 

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<Value>& args) {
   std::vector<nghttp2_settings_entry> 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<Value>& args) {
 // Used to fill in the spec defined initial values for each setting.
 void RefreshDefaultSettings(const FunctionCallbackInfo<Value>& 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<Value>& args) {
 template <get_setting fn>
 void RefreshSettings(const FunctionCallbackInfo<Value>& 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<Object>());
   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<Value>& args) {
 // Used to fill in the spec defined initial values for each setting.
 void RefreshSessionState(const FunctionCallbackInfo<Value>& 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<Object>());
   nghttp2_session* s = session->session();
@@ -308,7 +304,7 @@ void RefreshStreamState(const FunctionCallbackInfo<Value>& 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<Value>& args) {
 void Http2Session::SubmitSettings(const FunctionCallbackInfo<Value>& 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<nghttp2_settings_entry> entries;
@@ -1148,43 +1145,27 @@ void Initialize(Local<Object> 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<http2_state>(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);