From 41c7e96f1a81ea416ebb3ba45f2815e0202d6b75 Mon Sep 17 00:00:00 2001 From: Bert Belder 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 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/api.cc | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) (limited to 'core/libdeno/api.cc') diff --git a/core/libdeno/api.cc b/core/libdeno/api.cc index 53dad58e6..1110b1d34 100644 --- a/core/libdeno/api.cc +++ b/core/libdeno/api.cc @@ -46,7 +46,7 @@ Deno* deno_new(deno_config config) { } deno::DenoIsolate* d = new deno::DenoIsolate(config); v8::Isolate::CreateParams params; - params.array_buffer_allocator = d->array_buffer_allocator_; + params.array_buffer_allocator = &deno::ArrayBufferAllocator::global(); params.external_references = deno::external_references; if (config.load_snapshot.data_ptr) { @@ -148,12 +148,9 @@ void deno_execute(Deno* d_, void* user_data, const char* js_filename, deno::Execute(context, js_filename, js_source); } -void deno_zero_copy_release(Deno* d_, size_t zero_copy_id) { - auto* d = unwrap(d_); - v8::Isolate::Scope isolate_scope(d->isolate_); - v8::Locker locker(d->isolate_); - v8::HandleScope handle_scope(d->isolate_); - d->DeleteZeroCopyRef(zero_copy_id); +void deno_pinned_buf_delete(deno_pinned_buf* buf) { + // The PinnedBuf destructor implicitly releases the ArrayBuffer reference. + auto _ = deno::PinnedBuf(*buf); } void deno_respond(Deno* d_, void* user_data, deno_buf buf) { @@ -161,7 +158,6 @@ void deno_respond(Deno* d_, void* user_data, deno_buf buf) { if (d->current_args_ != nullptr) { // Synchronous response. if (buf.data_ptr != nullptr) { - DCHECK_EQ(buf.zero_copy_id, 0); auto ab = deno::ImportBuf(d, buf); d->current_args_->GetReturnValue().Set(ab); } @@ -189,9 +185,6 @@ void deno_respond(Deno* d_, void* user_data, deno_buf buf) { v8::Local args[1]; int argc = 0; - // You cannot use zero_copy_buf with deno_respond(). Use - // deno_zero_copy_release() instead. - DCHECK_EQ(buf.zero_copy_id, 0); if (buf.data_ptr != nullptr) { args[0] = deno::ImportBuf(d, buf); argc = 1; -- cgit v1.2.3