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.rs | 84 ++++++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 65 insertions(+), 19 deletions(-)

(limited to 'core/libdeno.rs')

diff --git a/core/libdeno.rs b/core/libdeno.rs
index 048db1311..4853c318e 100644
--- a/core/libdeno.rs
+++ b/core/libdeno.rs
@@ -4,9 +4,13 @@ use libc::c_char;
 use libc::c_int;
 use libc::c_void;
 use libc::size_t;
+use std::convert::From;
 use std::marker::PhantomData;
 use std::ops::{Deref, DerefMut};
+use std::option::Option;
 use std::ptr::null;
+use std::ptr::NonNull;
+use std::slice;
 
 // TODO(F001): change this definition to `extern { pub type isolate; }`
 // After RFC 1861 is stablized. See https://github.com/rust-lang/rust/issues/43467.
@@ -27,7 +31,6 @@ pub struct deno_buf {
   alloc_len: usize,
   data_ptr: *const u8,
   data_len: usize,
-  pub zero_copy_id: usize,
 }
 
 /// `deno_buf` can not clone, and there is no interior mutability.
@@ -42,7 +45,6 @@ impl deno_buf {
       alloc_len: 0,
       data_ptr: null(),
       data_len: 0,
-      zero_copy_id: 0,
     }
   }
 
@@ -53,7 +55,6 @@ impl deno_buf {
       alloc_len: 0,
       data_ptr: ptr,
       data_len: len,
-      zero_copy_id: 0,
     }
   }
 }
@@ -67,7 +68,6 @@ impl<'a> From<&'a [u8]> for deno_buf {
       alloc_len: 0,
       data_ptr: x.as_ref().as_ptr(),
       data_len: x.len(),
-      zero_copy_id: 0,
     }
   }
 }
@@ -80,35 +80,81 @@ impl Deref for deno_buf {
   }
 }
 
-impl DerefMut for deno_buf {
+impl AsRef<[u8]> for deno_buf {
   #[inline]
-  fn deref_mut(&mut self) -> &mut [u8] {
+  fn as_ref(&self) -> &[u8] {
+    &*self
+  }
+}
+
+/// A PinnedBuf encapsulates a slice that's been borrowed from a JavaScript
+/// ArrayBuffer object. JavaScript objects can normally be garbage collected,
+/// but the existence of a PinnedBuf inhibits this until it is dropped. It
+/// behaves much like an Arc<[u8]>, although a PinnedBuf currently can't be
+/// cloned.
+#[repr(C)]
+pub struct PinnedBuf {
+  data_ptr: NonNull<u8>,
+  data_len: usize,
+  pin: NonNull<c_void>,
+}
+
+#[repr(C)]
+pub struct PinnedBufRaw {
+  data_ptr: *mut u8,
+  data_len: usize,
+  pin: *mut c_void,
+}
+
+unsafe impl Send for PinnedBuf {}
+unsafe impl Send for PinnedBufRaw {}
+
+impl PinnedBuf {
+  pub fn new(raw: PinnedBufRaw) -> Option<Self> {
+    NonNull::new(raw.data_ptr).map(|data_ptr| PinnedBuf {
+      data_ptr,
+      data_len: raw.data_len,
+      pin: NonNull::new(raw.pin).unwrap(),
+    })
+  }
+}
+
+impl Drop for PinnedBuf {
+  fn drop(&mut self) {
     unsafe {
-      if self.alloc_ptr.is_null() {
-        panic!("Can't modify the buf");
-      }
-      std::slice::from_raw_parts_mut(self.data_ptr as *mut u8, self.data_len)
+      let raw = &mut *(self as *mut PinnedBuf as *mut PinnedBufRaw);
+      deno_pinned_buf_delete(raw);
     }
   }
 }
 
-impl AsRef<[u8]> for deno_buf {
-  #[inline]
+impl Deref for PinnedBuf {
+  type Target = [u8];
+  fn deref(&self) -> &[u8] {
+    unsafe { slice::from_raw_parts(self.data_ptr.as_ptr(), self.data_len) }
+  }
+}
+
+impl DerefMut for PinnedBuf {
+  fn deref_mut(&mut self) -> &mut [u8] {
+    unsafe { slice::from_raw_parts_mut(self.data_ptr.as_ptr(), self.data_len) }
+  }
+}
+
+impl AsRef<[u8]> for PinnedBuf {
   fn as_ref(&self) -> &[u8] {
     &*self
   }
 }
 
-impl AsMut<[u8]> for deno_buf {
-  #[inline]
+impl AsMut<[u8]> for PinnedBuf {
   fn as_mut(&mut self) -> &mut [u8] {
-    if self.alloc_ptr.is_null() {
-      panic!("Can't modify the buf");
-    }
     &mut *self
   }
 }
 
+pub use PinnedBufRaw as deno_pinned_buf;
+
 #[repr(C)]
 pub struct deno_snapshot<'a> {
   pub data_ptr: *const u8,
@@ -156,7 +202,7 @@ impl Snapshot2<'_> {
 type deno_recv_cb = unsafe extern "C" fn(
   user_data: *mut c_void,
   control_buf: deno_buf, // deprecated
-  zero_copy_buf: deno_buf,
+  zero_copy_buf: deno_pinned_buf,
 );
 
 #[allow(non_camel_case_types)]
@@ -220,7 +266,7 @@ extern "C" {
     user_data: *const c_void,
     buf: deno_buf,
   );
-  pub fn deno_zero_copy_release(i: *const isolate, zero_copy_id: usize);
+  pub fn deno_pinned_buf_delete(buf: &mut deno_pinned_buf);
   pub fn deno_execute(
     i: *const isolate,
     user_data: *const c_void,
-- 
cgit v1.2.3