src: allow not materializing ArrayBuffers from C++

Where appropriate, use a helper that wraps around `ArrayBufferView::Buffer()` or `ArrayBufferView::CopyContents()` rather than `Buffer::Data()`, as that may help to avoid materializing the underlying `ArrayBuffer` when reading small typed arrays from C++. This allows keeping the performance benefits of the faster creation of heap-allocated small typed arrays in many cases.
addaleax · Feb 25, 2019 · Feb 25, 2019 · Feb 25, 2019 · Feb 25, 2019 · Feb 25, 2019
commit 250dce31507fdc7afcee2ed8707128ad0402c6c6
diff --git a/src/node_crypto.cc b/src/node_crypto.cc
diff --git a/src/node_crypto.h b/src/node_crypto.h
@@ -335,7 +335,7 @@ class SSLWrap {
 
   ClientHelloParser hello_parser_;
 
-  Persistent<v8::Object> ocsp_response_;
+  Persistent<v8::ArrayBufferView> ocsp_response_;
   Persistent<v8::Value> sni_context_;
 
   friend class SecureContext;
@@ -464,7 +464,7 @@ class KeyObject : public BaseObject {
   static void New(const v8::FunctionCallbackInfo<v8::Value>& args);
 
   static void Init(const v8::FunctionCallbackInfo<v8::Value>& args);
-  void InitSecret(const char* key, size_t key_len);
+  void InitSecret(v8::Local<v8::ArrayBufferView> abv);
   void InitPublic(const ManagedEVPPKey& pkey);
   void InitPrivate(const ManagedEVPPKey& pkey);
 
@@ -805,8 +805,7 @@ class ECDH : public BaseObject {
   static void Initialize(Environment* env, v8::Local<v8::Object> target);
   static ECPointPointer BufferToPoint(Environment* env,
                                       const EC_GROUP* group,
-                                      char* data,
-                                      size_t len);
+                                      v8::Local<v8::Value> buf);
 
   // TODO(joyeecheung): track the memory used by OpenSSL types
   SET_NO_MEMORY_INFO()

diff --git a/src/node_http2.cc b/src/node_http2.cc
@@ -12,6 +12,7 @@
 namespace node {
 
 using v8::ArrayBuffer;
+using v8::ArrayBufferView;
 using v8::Boolean;
 using v8::Context;
 using v8::Float64Array;
@@ -2483,7 +2484,7 @@ void Http2Session::Request(const FunctionCallbackInfo<Value>& args) {
 // state of the Http2Session, it's simply a notification.
 void Http2Session::Goaway(uint32_t code,
                           int32_t lastStreamID,
-                          uint8_t* data,
+                          const uint8_t* data,
                           size_t len) {
   if (IsDestroyed())
     return;
@@ -2508,16 +2509,13 @@ void Http2Session::Goaway(const FunctionCallbackInfo<Value>& args) {
 
   uint32_t code = args[0]->Uint32Value(context).ToChecked();
   int32_t lastStreamID = args[1]->Int32Value(context).ToChecked();
-  Local<Value> opaqueData = args[2];
-  uint8_t* data = nullptr;
-  size_t length = 0;
+  ArrayBufferViewContents<uint8_t> opaque_data;
 
-  if (Buffer::HasInstance(opaqueData)) {
-    data = reinterpret_cast<uint8_t*>(Buffer::Data(opaqueData));
-    length = Buffer::Length(opaqueData);
+  if (args[2]->IsArrayBufferView()) {
+    opaque_data.Read(args[2].As<ArrayBufferView>());
   }
 
-  session->Goaway(code, lastStreamID, data, length);
+  session->Goaway(code, lastStreamID, opaque_data.data(), opaque_data.length());
 }
 
 // Update accounting of data chunks. This is used primarily to manage timeout
@@ -2771,10 +2769,10 @@ void Http2Session::Ping(const FunctionCallbackInfo<Value>& args) {
 
   // A PING frame may have exactly 8 bytes of payload data. If not provided,
   // then the current hrtime will be used as the payload.
-  uint8_t* payload = nullptr;
-  if (Buffer::HasInstance(args[0])) {
-    payload = reinterpret_cast<uint8_t*>(Buffer::Data(args[0]));
-    CHECK_EQ(Buffer::Length(args[0]), 8);
+  ArrayBufferViewContents<uint8_t, 8> payload;
+  if (args[0]->IsArrayBufferView()) {
+    payload.Read(args[0].As<ArrayBufferView>());
+    CHECK_EQ(payload.length(), 8);
   }
 
   Local<Object> obj;
@@ -2799,7 +2797,7 @@ void Http2Session::Ping(const FunctionCallbackInfo<Value>& args) {
   // the callback will be invoked and a notification sent out to JS land. The
   // notification will include the duration of the ping, allowing the round
   // trip to be measured.
-  ping->Send(payload);
+  ping->Send(payload.data());
   args.GetReturnValue().Set(true);
 }
 
@@ -2871,7 +2869,7 @@ Http2Session::Http2Ping::Http2Ping(Http2Session* session, Local<Object> obj)
       session_(session),
       startTime_(uv_hrtime()) {}
 
-void Http2Session::Http2Ping::Send(uint8_t* payload) {
+void Http2Session::Http2Ping::Send(const uint8_t* payload) {
   uint8_t data[8];
   if (payload == nullptr) {
     memcpy(&data, &startTime_, arraysize(data));

diff --git a/src/node_http2.h b/src/node_http2.h
@@ -699,7 +699,8 @@ class Http2Session : public AsyncWrap, public StreamListener {
   void Close(uint32_t code = NGHTTP2_NO_ERROR,
              bool socket_closed = false);
   void Consume(Local<External> external);
-  void Goaway(uint32_t code, int32_t lastStreamID, uint8_t* data, size_t len);
+  void Goaway(uint32_t code, int32_t lastStreamID,
+              const uint8_t* data, size_t len);
   void AltSvc(int32_t id,
               uint8_t* origin,
               size_t origin_len,
@@ -1089,7 +1090,7 @@ class Http2Session::Http2Ping : public AsyncWrap {
   SET_MEMORY_INFO_NAME(Http2Ping)
   SET_SELF_SIZE(Http2Ping)
 
-  void Send(uint8_t* payload);
+  void Send(const uint8_t* payload);
   void Done(bool ack, const uint8_t* payload = nullptr);
 
  private:

diff --git a/src/string_decoder.cc b/src/string_decoder.cc
@@ -4,6 +4,7 @@
 #include "string_decoder-inl.h"
 
 using v8::Array;
+using v8::ArrayBufferView;
 using v8::Context;
 using v8::FunctionCallbackInfo;
 using v8::Integer;
@@ -252,9 +253,13 @@ void DecodeData(const FunctionCallbackInfo<Value>& args) {
   StringDecoder* decoder =
       reinterpret_cast<StringDecoder*>(Buffer::Data(args[0]));
   CHECK_NOT_NULL(decoder);
-  size_t nread = Buffer::Length(args[1]);
+
+  CHECK(args[1]->IsArrayBufferView());
+  ArrayBufferViewContents<char> content(args[1].As<ArrayBufferView>());
+  size_t length = content.length();
+
   MaybeLocal<String> ret =
-      decoder->DecodeData(args.GetIsolate(), Buffer::Data(args[1]), &nread);
+      decoder->DecodeData(args.GetIsolate(), content.data(), &length);
   if (!ret.IsEmpty())
     args.GetReturnValue().Set(ret.ToLocalChecked());
 }

diff --git a/src/util-inl.h b/src/util-inl.h
@@ -466,6 +466,32 @@ SlicedArguments::SlicedArguments(
     (*this)[i] = args[i + start];
 }
 
+template <typename T, size_t S>
+ArrayBufferViewContents<T, S>::ArrayBufferViewContents(
+    v8::Local<v8::Value> value) {
+  CHECK(value->IsArrayBufferView());
+  Read(value.As<v8::ArrayBufferView>());
+}
+
+template <typename T, size_t S>
+ArrayBufferViewContents<T, S>::ArrayBufferViewContents(
+    v8::Local<v8::ArrayBufferView> abv) {
+  Read(abv);
+}
+
+template <typename T, size_t S>
+void ArrayBufferViewContents<T, S>::Read(v8::Local<v8::ArrayBufferView> abv) {
+  static_assert(sizeof(T) == 1, "Only supports one-byte data at the moment");
+  length_ = abv->ByteLength();
+  if (length_ > sizeof(stack_storage_) || abv->HasBuffer()) {
+    data_ = static_cast<T*>(abv->Buffer()->GetContents().Data()) +
+        abv->ByteOffset();
+  } else {
+    abv->CopyContents(stack_storage_, sizeof(stack_storage_));
+    data_ = stack_storage_;
+  }
+}
+
 }  // namespace node
 
 #endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

diff --git a/src/util.cc b/src/util.cc
@@ -29,6 +29,7 @@
 
 namespace node {
 
+using v8::ArrayBufferView;
 using v8::Isolate;
 using v8::Local;
 using v8::String;
@@ -89,11 +90,11 @@ BufferValue::BufferValue(Isolate* isolate, Local<Value> value) {
 
   if (value->IsString()) {
     MakeUtf8String(isolate, value, this);
-  } else if (Buffer::HasInstance(value)) {
-    const size_t len = Buffer::Length(value);
+  } else if (value->IsArrayBufferView()) {
+    const size_t len = value.As<ArrayBufferView>()->ByteLength();
     // Leave place for the terminating '\0' byte.
     AllocateSufficientStorage(len + 1);
-    memcpy(out(), Buffer::Data(value), len);
+    value.As<ArrayBufferView>()->CopyContents(out(), len);
     SetLengthAndZeroTerminate(len);
   } else {
     Invalidate();

diff --git a/src/util.h b/src/util.h
@@ -417,6 +417,27 @@ class MaybeStackBuffer {
   T buf_st_[kStackStorageSize];
 };
 
+// Provides access to an ArrayBufferView's storage, either the original,
+// or for small data, a copy of it. This object's lifetime is bound to the
+// original ArrayBufferView's lifetime.
+template <typename T, size_t kStackStorageSize = 64>
+class ArrayBufferViewContents {
+ public:
+  ArrayBufferViewContents() {}
+
+  explicit inline ArrayBufferViewContents(v8::Local<v8::Value> value);
+  explicit inline ArrayBufferViewContents(v8::Local<v8::ArrayBufferView> abv);
+  inline void Read(v8::Local<v8::ArrayBufferView> abv);
+
+  inline const T* data() const { return data_; }
+  inline size_t length() const { return length_; }
+
+ private:
+  T stack_storage_[kStackStorageSize];
+  T* data_ = nullptr;
+  size_t length_ = 0;
+};
+
 class Utf8Value : public MaybeStackBuffer<char> {
  public:
   explicit Utf8Value(v8::Isolate* isolate, v8::Local<v8::Value> value);