From 41c7e96f1a81ea416ebb3ba45f2815e0202d6b75 Mon Sep 17 00:00:00 2001
From: Bert Belder <bertbelder@gmail.com>
Date: Sun, 28 Apr 2019 21:31:10 +0200
Subject: Refactor zero-copy buffers for performance and to prevent memory
 leaks

* In order to prevent ArrayBuffers from getting garbage collected by V8,
  we used to store a v8::Persistent<ArrayBuffer> in a map. This patch
  introduces a custom ArrayBuffer allocator which doesn't use Persistent
  handles, but instead stores a pointer to the actual ArrayBuffer data
  alongside with a reference count. Since creating Persistent handles
  has quite a bit of overhead, this change significantly increases
  performance. Various HTTP server benchmarks report about 5-10% more
  requests per second than before.

* Previously the Persistent handle that prevented garbage collection had
  to be released manually, and this wasn't always done, which was
  causing memory leaks. This has been resolved by introducing a new
  `PinnedBuf` type in both Rust and C++ that automatically re-enables
  garbage collection when it goes out of scope.

* Zero-copy buffers are now correctly wrapped in an Option if there is a
  possibility that they're not present. This clears up a correctness
  issue where we were creating zero-length slices from a null pointer,
  which is against the rules.
---
 core/libdeno/internal.h | 26 +++-----------------------
 1 file changed, 3 insertions(+), 23 deletions(-)

(limited to 'core/libdeno/internal.h')

diff --git a/core/libdeno/internal.h b/core/libdeno/internal.h
index 0f4df9908..b75cc9717 100644
--- a/core/libdeno/internal.h
+++ b/core/libdeno/internal.h
@@ -6,6 +6,8 @@
 #include <string>
 #include <utility>
 #include <vector>
+
+#include "buffer.h"
 #include "deno.h"
 #include "third_party/v8/include/v8.h"
 #include "third_party/v8/src/base/logging.h"
@@ -36,11 +38,9 @@ class DenoIsolate {
         snapshot_creator_(nullptr),
         global_import_buf_ptr_(nullptr),
         recv_cb_(config.recv_cb),
-        next_zero_copy_id_(1),  // zero_copy_id must not be zero.
         user_data_(nullptr),
         resolve_cb_(nullptr),
         has_snapshotted_(false) {
-    array_buffer_allocator_ = v8::ArrayBuffer::Allocator::NewDefaultAllocator();
     if (config.load_snapshot.data_ptr) {
       snapshot_.data =
           reinterpret_cast<const char*>(config.load_snapshot.data_ptr);
@@ -65,7 +65,6 @@ class DenoIsolate {
     } else {
       isolate_->Dispose();
     }
-    delete array_buffer_allocator_;
   }
 
   static inline DenoIsolate* FromIsolate(v8::Isolate* isolate) {
@@ -89,31 +88,13 @@ class DenoIsolate {
     }
   }
 
-  void DeleteZeroCopyRef(size_t zero_copy_id) {
-    DCHECK_NE(zero_copy_id, 0);
-    // Delete persistent reference to data ArrayBuffer.
-    auto it = zero_copy_map_.find(zero_copy_id);
-    if (it != zero_copy_map_.end()) {
-      it->second.Reset();
-      zero_copy_map_.erase(it);
-    }
-  }
-
-  void AddZeroCopyRef(size_t zero_copy_id, v8::Local<v8::Value> zero_copy_v) {
-    zero_copy_map_.emplace(std::piecewise_construct,
-                           std::make_tuple(zero_copy_id),
-                           std::make_tuple(isolate_, zero_copy_v));
-  }
-
   v8::Isolate* isolate_;
   v8::Locker* locker_;
-  v8::ArrayBuffer::Allocator* array_buffer_allocator_;
   deno_buf shared_;
   const v8::FunctionCallbackInfo<v8::Value>* current_args_;
   v8::SnapshotCreator* snapshot_creator_;
   void* global_import_buf_ptr_;
   deno_recv_cb recv_cb_;
-  size_t next_zero_copy_id_;
   void* user_data_;
 
   std::map<deno_mod, ModuleInfo> mods_;
@@ -121,7 +102,6 @@ class DenoIsolate {
   deno_resolve_cb resolve_cb_;
 
   v8::Persistent<v8::Context> context_;
-  std::map<size_t, v8::Persistent<v8::Value>> zero_copy_map_;
   std::map<int, v8::Persistent<v8::Value>> pending_promise_map_;
   std::string last_exception_;
   v8::Persistent<v8::Function> recv_;
@@ -177,7 +157,7 @@ static intptr_t external_references[] = {
     reinterpret_cast<intptr_t>(MessageCallback),
     0};
 
-static const deno_buf empty_buf = {nullptr, 0, nullptr, 0, 0};
+static const deno_buf empty_buf = {nullptr, 0, nullptr, 0};
 static const deno_snapshot empty_snapshot = {nullptr, 0};
 
 Deno* NewFromSnapshot(void* user_data, deno_recv_cb cb);
-- 
cgit v1.2.3