src: move AliasedBuffer implementation to -inl.h

Drive-by: Replace the SFINAE with a static_assert because we don't
have (or need) an implementation for non-scalar AliasedBufferBase
otherwise. Add forward declarations to memory_tracker.h now that
aliased-buffer.h no longer includes util-inl.h.

PR-URL: https://github.com/nodejs/node/pull/46817
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

node.gyp

@@ -571,6 +571,7 @@
         'src/uv.cc',
         # headers to make for a more pleasant IDE experience
         'src/aliased_buffer.h',
+        'src/aliased_buffer-inl.h',
         'src/aliased_struct.h',
         'src/aliased_struct-inl.h',
         'src/async_wrap.h',

src/aliased_buffer-inl.h

@@ -0,0 +1,213 @@
+#ifndef SRC_ALIASED_BUFFER_INL_H_
+#define SRC_ALIASED_BUFFER_INL_H_
+
+#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
+
+#include "aliased_buffer.h"
+#include "util-inl.h"
+
+namespace node {
+
+typedef size_t AliasedBufferIndex;
+
+template <typename NativeT, typename V8T>
+AliasedBufferBase<NativeT, V8T>::AliasedBufferBase(
+    v8::Isolate* isolate, const size_t count, const AliasedBufferIndex* index)
+    : isolate_(isolate), count_(count), byte_offset_(0), index_(index) {
+  CHECK_GT(count, 0);
+  if (index != nullptr) {
+    // Will be deserialized later.
+    return;
+  }
+  const v8::HandleScope handle_scope(isolate_);
+  const size_t size_in_bytes =
+      MultiplyWithOverflowCheck(sizeof(NativeT), count);
+
+  // allocate v8 ArrayBuffer
+  v8::Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(isolate_, size_in_bytes);
+  buffer_ = static_cast<NativeT*>(ab->Data());
+
+  // allocate v8 TypedArray
+  v8::Local<V8T> js_array = V8T::New(ab, byte_offset_, count);
+  js_array_ = v8::Global<V8T>(isolate, js_array);
+}
+
+template <typename NativeT, typename V8T>
+AliasedBufferBase<NativeT, V8T>::AliasedBufferBase(
+    v8::Isolate* isolate,
+    const size_t byte_offset,
+    const size_t count,
+    const AliasedBufferBase<uint8_t, v8::Uint8Array>& backing_buffer,
+    const AliasedBufferIndex* index)
+    : isolate_(isolate),
+      count_(count),
+      byte_offset_(byte_offset),
+      index_(index) {
+  if (index != nullptr) {
+    // Will be deserialized later.
+    return;
+  }
+  const v8::HandleScope handle_scope(isolate_);
+  v8::Local<v8::ArrayBuffer> ab = backing_buffer.GetArrayBuffer();
+
+  // validate that the byte_offset is aligned with sizeof(NativeT)
+  CHECK_EQ(byte_offset & (sizeof(NativeT) - 1), 0);
+  // validate this fits inside the backing buffer
+  CHECK_LE(MultiplyWithOverflowCheck(sizeof(NativeT), count),
+           ab->ByteLength() - byte_offset);
+
+  buffer_ = reinterpret_cast<NativeT*>(
+      const_cast<uint8_t*>(backing_buffer.GetNativeBuffer() + byte_offset));
+
+  v8::Local<V8T> js_array = V8T::New(ab, byte_offset, count);
+  js_array_ = v8::Global<V8T>(isolate, js_array);
+}
+
+template <typename NativeT, typename V8T>
+AliasedBufferBase<NativeT, V8T>::AliasedBufferBase(
+    const AliasedBufferBase& that)
+    : isolate_(that.isolate_),
+      count_(that.count_),
+      byte_offset_(that.byte_offset_),
+      buffer_(that.buffer_) {
+  DCHECK_NULL(index_);
+  js_array_ = v8::Global<V8T>(that.isolate_, that.GetJSArray());
+}
+
+template <typename NativeT, typename V8T>
+AliasedBufferIndex AliasedBufferBase<NativeT, V8T>::Serialize(
+    v8::Local<v8::Context> context, v8::SnapshotCreator* creator) {
+  DCHECK_NULL(index_);
+  return creator->AddData(context, GetJSArray());
+}
+
+template <typename NativeT, typename V8T>
+inline void AliasedBufferBase<NativeT, V8T>::Deserialize(
+    v8::Local<v8::Context> context) {
+  DCHECK_NOT_NULL(index_);
+  v8::Local<V8T> arr =
+      context->GetDataFromSnapshotOnce<V8T>(*index_).ToLocalChecked();
+  // These may not hold true for AliasedBuffers that have grown, so should
+  // be removed when we expand the snapshot support.
+  DCHECK_EQ(count_, arr->Length());
+  DCHECK_EQ(byte_offset_, arr->ByteOffset());
+  uint8_t* raw = static_cast<uint8_t*>(arr->Buffer()->Data());
+  buffer_ = reinterpret_cast<NativeT*>(raw + byte_offset_);
+  js_array_.Reset(isolate_, arr);
+  index_ = nullptr;
+}
+
+template <typename NativeT, typename V8T>
+AliasedBufferBase<NativeT, V8T>& AliasedBufferBase<NativeT, V8T>::operator=(
+    AliasedBufferBase<NativeT, V8T>&& that) noexcept {
+  DCHECK_NULL(index_);
+  this->~AliasedBufferBase();
+  isolate_ = that.isolate_;
+  count_ = that.count_;
+  byte_offset_ = that.byte_offset_;
+  buffer_ = that.buffer_;
+
+  js_array_.Reset(isolate_, that.js_array_.Get(isolate_));
+
+  that.buffer_ = nullptr;
+  that.js_array_.Reset();
+  return *this;
+}
+
+template <typename NativeT, typename V8T>
+v8::Local<V8T> AliasedBufferBase<NativeT, V8T>::GetJSArray() const {
+  DCHECK_NULL(index_);
+  return js_array_.Get(isolate_);
+}
+
+template <typename NativeT, typename V8T>
+void AliasedBufferBase<NativeT, V8T>::Release() {
+  DCHECK_NULL(index_);
+  js_array_.Reset();
+}
+
+template <typename NativeT, typename V8T>
+v8::Local<v8::ArrayBuffer> AliasedBufferBase<NativeT, V8T>::GetArrayBuffer()
+    const {
+  return GetJSArray()->Buffer();
+}
+
+template <typename NativeT, typename V8T>
+inline const NativeT* AliasedBufferBase<NativeT, V8T>::GetNativeBuffer() const {
+  DCHECK_NULL(index_);
+  return buffer_;
+}
+
+template <typename NativeT, typename V8T>
+inline const NativeT* AliasedBufferBase<NativeT, V8T>::operator*() const {
+  return GetNativeBuffer();
+}
+
+template <typename NativeT, typename V8T>
+inline void AliasedBufferBase<NativeT, V8T>::SetValue(const size_t index,
+                                                      NativeT value) {
+  DCHECK_LT(index, count_);
+  DCHECK_NULL(index_);
+  buffer_[index] = value;
+}
+
+template <typename NativeT, typename V8T>
+inline const NativeT AliasedBufferBase<NativeT, V8T>::GetValue(
+    const size_t index) const {
+  DCHECK_NULL(index_);
+  DCHECK_LT(index, count_);
+  return buffer_[index];
+}
+
+template <typename NativeT, typename V8T>
+typename AliasedBufferBase<NativeT, V8T>::Reference
+AliasedBufferBase<NativeT, V8T>::operator[](size_t index) {
+  DCHECK_NULL(index_);
+  return Reference(this, index);
+}
+
+template <typename NativeT, typename V8T>
+NativeT AliasedBufferBase<NativeT, V8T>::operator[](size_t index) const {
+  return GetValue(index);
+}
+
+template <typename NativeT, typename V8T>
+size_t AliasedBufferBase<NativeT, V8T>::Length() const {
+  return count_;
+}
+
+template <typename NativeT, typename V8T>
+void AliasedBufferBase<NativeT, V8T>::reserve(size_t new_capacity) {
+  DCHECK_NULL(index_);
+  DCHECK_GE(new_capacity, count_);
+  DCHECK_EQ(byte_offset_, 0);
+  const v8::HandleScope handle_scope(isolate_);
+
+  const size_t old_size_in_bytes = sizeof(NativeT) * count_;
+  const size_t new_size_in_bytes =
+      MultiplyWithOverflowCheck(sizeof(NativeT), new_capacity);
+
+  // allocate v8 new ArrayBuffer
+  v8::Local<v8::ArrayBuffer> ab =
+      v8::ArrayBuffer::New(isolate_, new_size_in_bytes);
+
+  // allocate new native buffer
+  NativeT* new_buffer = static_cast<NativeT*>(ab->Data());
+  // copy old content
+  memcpy(new_buffer, buffer_, old_size_in_bytes);
+
+  // allocate v8 TypedArray
+  v8::Local<V8T> js_array = V8T::New(ab, byte_offset_, new_capacity);
+
+  // move over old v8 TypedArray
+  js_array_ = std::move(v8::Global<V8T>(isolate_, js_array));
+
+  buffer_ = new_buffer;
+  count_ = new_capacity;
+}
+
+}  // namespace node
+
+#endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
+
+#endif  // SRC_ALIASED_BUFFER_INL_H_

src/aliased_buffer.h

@@ -4,7 +4,6 @@
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
 #include <cinttypes>
-#include "util-inl.h"
 #include "v8.h"
 
 namespace node {
@@ -28,34 +27,14 @@ typedef size_t AliasedBufferIndex;
  * The encapsulation herein provides a placeholder where such writes can be
  * observed. Any notification APIs will be left as a future exercise.
  */
-template <class NativeT,
+template <class NativeT, class V8T>
-          class V8T,
-          // SFINAE NativeT to be scalar
-          typename = std::enable_if_t<std::is_scalar<NativeT>::value>>
 class AliasedBufferBase {
  public:
+  static_assert(std::is_scalar<NativeT>::value);
+
   AliasedBufferBase(v8::Isolate* isolate,
                     const size_t count,
-                    const AliasedBufferIndex* index = nullptr)
+                    const AliasedBufferIndex* index = nullptr);
-      : isolate_(isolate), count_(count), byte_offset_(0), index_(index) {
-    CHECK_GT(count, 0);
-    if (index != nullptr) {
-      // Will be deserialized later.
-      return;
-    }
-    const v8::HandleScope handle_scope(isolate_);
-    const size_t size_in_bytes =
-        MultiplyWithOverflowCheck(sizeof(NativeT), count);
-
-    // allocate v8 ArrayBuffer
-    v8::Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(
-        isolate_, size_in_bytes);
-    buffer_ = static_cast<NativeT*>(ab->Data());
-
-    // allocate v8 TypedArray
-    v8::Local<V8T> js_array = V8T::New(ab, byte_offset_, count);
-    js_array_ = v8::Global<V8T>(isolate, js_array);
-  }
 
   /**
    * Create an AliasedBufferBase over a sub-region of another aliased buffer.
@@ -71,74 +50,16 @@ class AliasedBufferBase {
       const size_t byte_offset,
       const size_t count,
       const AliasedBufferBase<uint8_t, v8::Uint8Array>& backing_buffer,
-      const AliasedBufferIndex* index = nullptr)
+      const AliasedBufferIndex* index = nullptr);
-      : isolate_(isolate),
-        count_(count),
-        byte_offset_(byte_offset),
-        index_(index) {
-    if (index != nullptr) {
-      // Will be deserialized later.
-      return;
-    }
-    const v8::HandleScope handle_scope(isolate_);
-    v8::Local<v8::ArrayBuffer> ab = backing_buffer.GetArrayBuffer();
 
-    // validate that the byte_offset is aligned with sizeof(NativeT)
+  AliasedBufferBase(const AliasedBufferBase& that);
-    CHECK_EQ(byte_offset & (sizeof(NativeT) - 1), 0);
-    // validate this fits inside the backing buffer
-    CHECK_LE(MultiplyWithOverflowCheck(sizeof(NativeT), count),
-             ab->ByteLength() - byte_offset);
-
-    buffer_ = reinterpret_cast<NativeT*>(
-        const_cast<uint8_t*>(backing_buffer.GetNativeBuffer() + byte_offset));
-
-    v8::Local<V8T> js_array = V8T::New(ab, byte_offset, count);
-    js_array_ = v8::Global<V8T>(isolate, js_array);
-  }
-
-  AliasedBufferBase(const AliasedBufferBase& that)
-      : isolate_(that.isolate_),
-        count_(that.count_),
-        byte_offset_(that.byte_offset_),
-        buffer_(that.buffer_) {
-    DCHECK_NULL(index_);
-    js_array_ = v8::Global<V8T>(that.isolate_, that.GetJSArray());
-  }
 
   AliasedBufferIndex Serialize(v8::Local<v8::Context> context,
-                              v8::SnapshotCreator* creator) {
+                               v8::SnapshotCreator* creator);
-    DCHECK_NULL(index_);
-    return creator->AddData(context, GetJSArray());
-  }
 
-  inline void Deserialize(v8::Local<v8::Context> context) {
+  inline void Deserialize(v8::Local<v8::Context> context);
-    DCHECK_NOT_NULL(index_);
-    v8::Local<V8T> arr =
-        context->GetDataFromSnapshotOnce<V8T>(*index_).ToLocalChecked();
-    // These may not hold true for AliasedBuffers that have grown, so should
-    // be removed when we expand the snapshot support.
-    DCHECK_EQ(count_, arr->Length());
-    DCHECK_EQ(byte_offset_, arr->ByteOffset());
-    uint8_t* raw = static_cast<uint8_t*>(arr->Buffer()->Data());
-    buffer_ = reinterpret_cast<NativeT*>(raw + byte_offset_);
-    js_array_.Reset(isolate_, arr);
-    index_ = nullptr;
-  }
 
-  AliasedBufferBase& operator=(AliasedBufferBase&& that) noexcept {
+  AliasedBufferBase& operator=(AliasedBufferBase&& that) noexcept;
-    DCHECK_NULL(index_);
-    this->~AliasedBufferBase();
-    isolate_ = that.isolate_;
-    count_ = that.count_;
-    byte_offset_ = that.byte_offset_;
-    buffer_ = that.buffer_;
-
-    js_array_.Reset(isolate_, that.js_array_.Get(isolate_));
-
-    that.buffer_ = nullptr;
-    that.js_array_.Reset();
-    return *this;
-  }
 
   /**
    * Helper class that is returned from operator[] to support assignment into
@@ -191,105 +112,50 @@ class AliasedBufferBase {
   /**
    *  Get the underlying v8 TypedArray overlayed on top of the native buffer
    */
-  v8::Local<V8T> GetJSArray() const {
+  v8::Local<V8T> GetJSArray() const;
-    DCHECK_NULL(index_);
-    return js_array_.Get(isolate_);
-  }
 
-  void Release() {
+  void Release();
-    DCHECK_NULL(index_);
-    js_array_.Reset();
-  }
 
   /**
   *  Get the underlying v8::ArrayBuffer underlying the TypedArray and
   *  overlaying the native buffer
   */
-  v8::Local<v8::ArrayBuffer> GetArrayBuffer() const {
+  v8::Local<v8::ArrayBuffer> GetArrayBuffer() const;
-    return GetJSArray()->Buffer();
-  }
 
   /**
    *  Get the underlying native buffer. Note that all reads/writes should occur
    *  through the GetValue/SetValue/operator[] methods
    */
-  inline const NativeT* GetNativeBuffer() const {
+  inline const NativeT* GetNativeBuffer() const;
-    DCHECK_NULL(index_);
-    return buffer_;
-  }
 
   /**
    *  Synonym for GetBuffer()
    */
-  inline const NativeT* operator * () const {
+  inline const NativeT* operator*() const;
-    return GetNativeBuffer();
-  }
 
   /**
    *  Set position index to given value.
    */
-  inline void SetValue(const size_t index, NativeT value) {
+  inline void SetValue(const size_t index, NativeT value);
-    DCHECK_LT(index, count_);
-    DCHECK_NULL(index_);
-    buffer_[index] = value;
-  }
 
   /**
    *  Get value at position index
    */
-  inline const NativeT GetValue(const size_t index) const {
+  inline const NativeT GetValue(const size_t index) const;
-    DCHECK_NULL(index_);
-    DCHECK_LT(index, count_);
-    return buffer_[index];
-  }
 
   /**
    *  Effectively, a synonym for GetValue/SetValue
    */
-  Reference operator[](size_t index) {
+  Reference operator[](size_t index);
-    DCHECK_NULL(index_);
-    return Reference(this, index);
-  }
 
-  NativeT operator[](size_t index) const {
+  NativeT operator[](size_t index) const;
-    return GetValue(index);
-  }
 
-  size_t Length() const {
+  size_t Length() const;
-    return count_;
-  }
 
   // Should only be used to extend the array.
   // Should only be used on an owning array, not one created as a sub array of
   // an owning `AliasedBufferBase`.
-  void reserve(size_t new_capacity) {
+  void reserve(size_t new_capacity);
-    DCHECK_NULL(index_);
-    DCHECK_GE(new_capacity, count_);
-    DCHECK_EQ(byte_offset_, 0);
-    const v8::HandleScope handle_scope(isolate_);
-
-    const size_t old_size_in_bytes = sizeof(NativeT) * count_;
-    const size_t new_size_in_bytes = MultiplyWithOverflowCheck(sizeof(NativeT),
-                                                              new_capacity);
-
-    // allocate v8 new ArrayBuffer
-    v8::Local<v8::ArrayBuffer> ab = v8::ArrayBuffer::New(
-        isolate_, new_size_in_bytes);
-
-    // allocate new native buffer
-    NativeT* new_buffer = static_cast<NativeT*>(ab->Data());
-    // copy old content
-    memcpy(new_buffer, buffer_, old_size_in_bytes);
-
-    // allocate v8 TypedArray
-    v8::Local<V8T> js_array = V8T::New(ab, byte_offset_, new_capacity);
-
-    // move over old v8 TypedArray
-    js_array_ = std::move(v8::Global<V8T>(isolate_, js_array));
-
-    buffer_ = new_buffer;
-    count_ = new_capacity;
-  }
 
  private:
   v8::Isolate* isolate_ = nullptr;
@@ -302,11 +168,22 @@ class AliasedBufferBase {
   const AliasedBufferIndex* index_ = nullptr;
 };
 
-typedef AliasedBufferBase<int32_t, v8::Int32Array> AliasedInt32Array;
+#define ALIASED_BUFFER_LIST(V)                                                 \
-typedef AliasedBufferBase<uint8_t, v8::Uint8Array> AliasedUint8Array;
+  V(int8_t, Int8Array)                                                         \
-typedef AliasedBufferBase<uint32_t, v8::Uint32Array> AliasedUint32Array;
+  V(uint8_t, Uint8Array)                                                       \
-typedef AliasedBufferBase<double, v8::Float64Array> AliasedFloat64Array;
+  V(int16_t, Int16Array)                                                       \
-typedef AliasedBufferBase<int64_t, v8::BigInt64Array> AliasedBigInt64Array;
+  V(uint16_t, Uint16Array)                                                     \
+  V(int32_t, Int32Array)                                                       \
+  V(uint32_t, Uint32Array)                                                     \
+  V(float, Float32Array)                                                       \
+  V(double, Float64Array)                                                      \
+  V(int64_t, BigInt64Array)
+
+#define V(NativeT, V8T)                                                        \
+  typedef AliasedBufferBase<NativeT, v8::V8T> Aliased##V8T;
+ALIASED_BUFFER_LIST(V)
+#undef V
+
 }  // namespace node
 
 #endif  // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS

src/env-inl.h

@@ -24,7 +24,7 @@
 
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
-#include "aliased_buffer.h"
+#include "aliased_buffer-inl.h"
 #include "callback_queue-inl.h"
 #include "env.h"
 #include "node.h"

src/memory_tracker-inl.h

@@ -3,6 +3,7 @@
 
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
+#include "aliased_buffer-inl.h"
 #include "memory_tracker.h"
 
 namespace node {

src/memory_tracker.h

@@ -13,8 +13,15 @@
 #include <string>
 #include <unordered_map>
 
+namespace v8 {
+class BackingStore;
+}
+
 namespace node {
 
+template <typename T>
+struct MallocedBuffer;
+
 // Set the node name of a MemoryRetainer to klass
 #define SET_MEMORY_INFO_NAME(Klass)                                            \
   inline const char* MemoryInfoName() const override { return #Klass; }

src/node_file.cc

@@ -19,7 +19,7 @@
 // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 #include "node_file.h"  // NOLINT(build/include_inline)
-#include "aliased_buffer.h"
+#include "aliased_buffer-inl.h"
 #include "memory_tracker-inl.h"
 #include "node_buffer.h"
 #include "node_external_reference.h"

src/node_http2.cc

@@ -1,5 +1,5 @@
 #include "node_http2.h"
-#include "aliased_buffer.h"
+#include "aliased_buffer-inl.h"
 #include "aliased_struct-inl.h"
 #include "debug_utils-inl.h"
 #include "histogram-inl.h"

src/node_perf.cc

@@ -1,5 +1,5 @@
 #include "node_perf.h"
-#include "aliased_buffer.h"
+#include "aliased_buffer-inl.h"
 #include "env-inl.h"
 #include "histogram-inl.h"
 #include "memory_tracker-inl.h"

src/node_v8.cc

@@ -20,6 +20,7 @@
 // USE OR OTHER DEALINGS IN THE SOFTWARE.
 
 #include "node_v8.h"
+#include "aliased_buffer-inl.h"
 #include "base_object-inl.h"
 #include "env-inl.h"
 #include "memory_tracker-inl.h"

test/cctest/test_aliased_buffer.cc

@@ -1,6 +1,6 @@
-#include "v8.h"
+#include "aliased_buffer-inl.h"
-#include "aliased_buffer.h"
 #include "node_test_fixture.h"
+#include "v8.h"
 
 using node::AliasedBufferBase;