From 0446e463c8f44b7992d1c14891bbcef96ca43eb4 Mon Sep 17 00:00:00 2001 From: LongYinan Date: Mon, 21 Dec 2020 16:10:30 +0800 Subject: [PATCH] fix(napi): memory issues in create_external_buffer --- bench/Cargo.toml | 6 ++++++ bench/bench.ts | 8 +++++++- bench/buffer.ts | 23 +++++++++++++++++++++++ bench/src/buffer.rs | 16 ++++++++++++++++ bench/src/lib.rs | 10 ++++++++++ bench/tsconfig.json | 3 ++- napi/src/env.rs | 20 ++++++++++++-------- 7 files changed, 76 insertions(+), 10 deletions(-) create mode 100644 bench/buffer.ts create mode 100644 bench/src/buffer.rs diff --git a/bench/Cargo.toml b/bench/Cargo.toml index a824ffc2..245b1344 100644 --- a/bench/Cargo.toml +++ b/bench/Cargo.toml @@ -11,6 +11,12 @@ crate-type = ["cdylib"] napi = {path = "../napi"} napi-derive = {path = "../napi-derive"} +[target.'cfg(all(unix, not(target_env = "musl"), not(target_arch = "aarch64")))'.dependencies] +jemallocator = {version = "0.3", features = ["disable_initial_exec_tls"]} + +[target.'cfg(windows)'.dependencies] +mimalloc = {version = "0.1"} + [build-dependencies] napi-build = {path = "../build"} diff --git a/bench/bench.ts b/bench/bench.ts index 70e16588..eb9d2cc0 100644 --- a/bench/bench.ts +++ b/bench/bench.ts @@ -4,11 +4,17 @@ import { join } from 'path' import { Summary } from 'benny/lib/internal/common-types' import { benchAsync } from './async' +import { benchBuffer } from './buffer' import { benchNoop } from './noop' import { benchPlus } from './plus' async function run() { - const output = [await benchNoop(), await benchPlus(), await benchAsync()] + const output = [ + await benchNoop(), + await benchPlus(), + await benchBuffer(), + await benchAsync(), + ] .map(formatSummary) .join('\n') diff --git a/bench/buffer.ts b/bench/buffer.ts new file mode 100644 index 00000000..24554c68 --- /dev/null +++ b/bench/buffer.ts @@ -0,0 +1,23 @@ +import b from 'benny' + +const { benchCreateBuffer } = require('./index.node') + +function createBuffer() { + const buf = Buffer.alloc(100000) + buf[0] = 1 + buf[1] = 2 + return buf +} + +export const benchBuffer = () => + b.suite( + 'Create buffer', + b.add('napi-rs', () => { + benchCreateBuffer() + }), + b.add('JavaScript', () => { + createBuffer() + }), + b.cycle(), + b.complete(), + ) diff --git a/bench/src/buffer.rs b/bench/src/buffer.rs new file mode 100644 index 00000000..f92fabff --- /dev/null +++ b/bench/src/buffer.rs @@ -0,0 +1,16 @@ +use napi::{ContextlessResult, Env, JsBuffer, JsObject, Result}; + +#[contextless_function] +pub fn bench_create_buffer(env: Env) -> ContextlessResult { + let mut output = Vec::with_capacity(100000); + output.push(1); + output.push(2); + env + .create_buffer_with_data(output) + .map(|v| Some(v.into_raw())) +} + +pub fn register_js(exports: &mut JsObject) -> Result<()> { + exports.create_named_method("benchCreateBuffer", bench_create_buffer)?; + Ok(()) +} diff --git a/bench/src/lib.rs b/bench/src/lib.rs index 653c7f5c..ec4badc8 100644 --- a/bench/src/lib.rs +++ b/bench/src/lib.rs @@ -3,7 +3,16 @@ extern crate napi_derive; use napi::{JsObject, Result}; +#[cfg(all(unix, not(target_env = "musl"), not(target_arch = "aarch64")))] +#[global_allocator] +static ALLOC: jemallocator::Jemalloc = jemallocator::Jemalloc; + +#[cfg(windows)] +#[global_allocator] +static ALLOC: mimalloc::MiMalloc = mimalloc::MiMalloc; + mod async_compute; +mod buffer; mod noop; mod plus; @@ -12,6 +21,7 @@ fn init(mut exports: JsObject) -> Result<()> { exports.create_named_method("noop", noop::noop)?; async_compute::register_js(&mut exports)?; + buffer::register_js(&mut exports)?; plus::register_js(&mut exports)?; Ok(()) diff --git a/bench/tsconfig.json b/bench/tsconfig.json index 215b3b0d..d6ba85d0 100644 --- a/bench/tsconfig.json +++ b/bench/tsconfig.json @@ -1,7 +1,8 @@ { "extends": "../tsconfig.json", "compilerOptions": { - "target": "ESNext" + "target": "ESNext", + "rootDir": "." }, "include": ["."], "exclude": ["target", "src"] diff --git a/napi/src/env.rs b/napi/src/env.rs index 4bff1702..0f49e56f 100644 --- a/napi/src/env.rs +++ b/napi/src/env.rs @@ -284,7 +284,7 @@ impl Env { /// This API allocates a node::Buffer object and initializes it with data backed by the passed in buffer. /// While this is still a fully-supported data structure, in most cases using a TypedArray will suffice. pub fn create_buffer_with_data(&self, mut data: Vec) -> Result { - let mut length = data.len(); + let length = data.len(); let mut raw_value = ptr::null_mut(); let data_ptr = data.as_mut_ptr(); check_status!(unsafe { @@ -293,7 +293,7 @@ impl Env { length, data_ptr as *mut c_void, Some(drop_buffer), - &mut length as *mut usize as *mut _, + Box::into_raw(Box::new((length, data.capacity()))) as *mut c_void, &mut raw_value, ) })?; @@ -403,7 +403,7 @@ impl Env { #[inline] pub fn create_arraybuffer_with_data(&self, data: Vec) -> Result { - let mut length = data.len(); + let length = data.len(); let mut raw_value = ptr::null_mut(); let data_ptr = data.as_ptr(); check_status!(unsafe { @@ -412,7 +412,7 @@ impl Env { data_ptr as *mut c_void, length, Some(drop_buffer), - &mut length as *mut usize as *mut c_void, + Box::into_raw(Box::new((length, data.capacity()))) as *mut c_void, &mut raw_value, ) })?; @@ -1023,10 +1023,14 @@ impl Env { } } -unsafe extern "C" fn drop_buffer(env: sys::napi_env, finalize_data: *mut c_void, len: *mut c_void) { - let length = len as *mut u64; - let length = *length as usize; - let _ = Vec::from_raw_parts(finalize_data as *mut u8, length, length); +unsafe extern "C" fn drop_buffer( + env: sys::napi_env, + finalize_data: *mut c_void, + hint: *mut c_void, +) { + let length_ptr = hint as *mut (usize, usize); + let (length, cap) = *Box::from_raw(length_ptr); + mem::drop(Vec::from_raw_parts(finalize_data as *mut u8, length, cap)); let mut changed = 0; let adjust_external_memory_status = sys::napi_adjust_external_memory(env, -(length as i64), &mut changed);