diff options
author | Bert Belder <bertbelder@gmail.com> | 2019-04-28 21:31:10 +0200 |
---|---|---|
committer | Bert Belder <bertbelder@gmail.com> | 2019-05-01 21:11:09 +0200 |
commit | 41c7e96f1a81ea416ebb3ba45f2815e0202d6b75 (patch) | |
tree | 5fcb15b8664d7579cf4db76d16754c8efa7c4667 /core/libdeno.rs | |
parent | abdb98a2516a9d6ec313805dffbc2107d38f8ed4 (diff) |
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.
Diffstat (limited to 'core/libdeno.rs')
-rw-r--r-- | core/libdeno.rs | 84 |
1 files changed, 65 insertions, 19 deletions
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, |