perf(ext/web): optimize transferArrayBuffer (denoland#16294)

Avoid copying enqueued data + misc optimizations to skip webidl converter.
DavidDeSimone · Oct 26, 2022 · 6cd9343 · 6cd9343
1 parent 046ab7d
commit 6cd9343
Show file tree

Hide file tree

Showing 3 changed files with 49 additions and 26 deletions.
diff --git a/ext/web/06_streams.js b/ext/web/06_streams.js
@@ -60,6 +60,7 @@
     WeakMapPrototypeSet,
   } = globalThis.__bootstrap.primordials;
   const consoleInternal = window.__bootstrap.console;
+  const ops = core.ops;
   const { AssertionError, assert } = window.__bootstrap.infra;
 
   /** @template T */
@@ -224,15 +225,9 @@
    * @returns {ArrayBufferLike}
    */
   function transferArrayBuffer(O) {
-    assert(!isDetachedBuffer(O));
-    const transferredIshVersion = O.slice(0);
-    ObjectDefineProperty(O, "byteLength", {
-      get() {
-        return 0;
-      },
-    });
+    const v = ops.op_transfer_arraybuffer(O);
     O[isFakeDetached] = true;
-    return transferredIshVersion;
+    return v;
   }
 
   /**
@@ -4620,26 +4615,32 @@
      * @param {UnderlyingSource<R>=} underlyingSource
      * @param {QueuingStrategy<R>=} strategy
      */
-    constructor(underlyingSource = undefined, strategy = {}) {
+    constructor(underlyingSource = undefined, strategy = undefined) {
       const prefix = "Failed to construct 'ReadableStream'";
       if (underlyingSource !== undefined) {
         underlyingSource = webidl.converters.object(underlyingSource, {
           prefix,
           context: "Argument 1",
         });
+      } else {
+        underlyingSource = null;
+      }
+      if (strategy !== undefined) {
+        strategy = webidl.converters.QueuingStrategy(strategy, {
+          prefix,
+          context: "Argument 2",
+        });
+      } else {
+        strategy = {};
       }
-      strategy = webidl.converters.QueuingStrategy(strategy, {
-        prefix,
-        context: "Argument 2",
-      });
       this[webidl.brand] = webidl.brand;
-      if (underlyingSource === undefined) {
-        underlyingSource = null;
+      let underlyingSourceDict = {};
+      if (underlyingSource !== undefined) {
+        underlyingSourceDict = webidl.converters.UnderlyingSource(
+          underlyingSource,
+          { prefix, context: "underlyingSource" },
+        );
       }
-      const underlyingSourceDict = webidl.converters.UnderlyingSource(
-        underlyingSource,
-        { prefix, context: "underlyingSource" },
-      );
       initializeReadableStream(this);
       if (underlyingSourceDict.type === "bytes") {
         if (strategy.size !== undefined) {
@@ -4700,13 +4701,17 @@
      * @param {ReadableStreamGetReaderOptions=} options
      * @returns {ReadableStreamDefaultReader<R> | ReadableStreamBYOBReader}
      */
-    getReader(options = {}) {
+    getReader(options = undefined) {
       webidl.assertBranded(this, ReadableStreamPrototype);
       const prefix = "Failed to execute 'getReader' on 'ReadableStream'";
-      options = webidl.converters.ReadableStreamGetReaderOptions(options, {
-        prefix,
-        context: "Argument 1",
-      });
+      if (options !== undefined) {
+        options = webidl.converters.ReadableStreamGetReaderOptions(options, {
+          prefix,
+          context: "Argument 1",
+        });
+      } else {
+        options = {};
+      }
       if (options.mode === undefined) {
         return acquireReadableStreamDefaultReader(this);
       } else {

diff --git a/ext/web/lib.rs b/ext/web/lib.rs
@@ -112,6 +112,7 @@ pub fn init<P: TimersPermission + 'static>(
       op_timer_handle::decl(),
       op_cancel_handle::decl(),
       op_sleep::decl(),
+      op_transfer_arraybuffer::decl(),
     ])
     .state(move |state| {
       state.put(blob_store.clone());
@@ -338,6 +339,23 @@ fn op_encoding_encode_into(
   Ok(())
 }
 
+#[op(v8)]
+fn op_transfer_arraybuffer<'a>(
+  scope: &mut v8::HandleScope<'a>,
+  input: serde_v8::Value<'a>,
+) -> Result<serde_v8::Value<'a>, AnyError> {
+  let ab = v8::Local::<v8::ArrayBuffer>::try_from(input.v8_value)?;
+  if !ab.is_detachable() {
+    return Err(type_error("ArrayBuffer is not detachable"));
+  }
+  let bs = ab.get_backing_store();
+  ab.detach();
+  let ab = v8::ArrayBuffer::with_backing_store(scope, &bs);
+  Ok(serde_v8::Value {
+    v8_value: ab.into(),
+  })
+}
+
 #[op]
 fn op_encode_binary_string(s: &[u8]) -> ByteString {
   ByteString::from(s)

diff --git a/tools/wpt/expectation.json b/tools/wpt/expectation.json
@@ -1302,8 +1302,8 @@
         "ReadableStream with byte source: Respond to multiple pull() by separate enqueue()",
         "ReadableStream with byte source: enqueue() discards auto-allocated BYOB request"
       ],
-      "non-transferable-buffers.any.html": false,
-      "non-transferable-buffers.any.worker.html": false,
+      "non-transferable-buffers.any.html": true,
+      "non-transferable-buffers.any.worker.html": true,
       "enqueue-with-detached-buffer.window.html": false,
       "tee.any.html": true,
       "tee.any.worker.html": true,