Repository: denoland/deno
Pull request: perf(ext/webstorage): use object wrap for `Storage`


![image](https://github.com/user-attachments/assets/3f86e2fd-9026-4965-8f3b-512423362f1e)


Depends on:
- https://github.com/denoland/deno_core/pull/970
- https://github.com/denoland/deno_core/pull/976
- https://github.com/denoland/deno_core/pull/980
- https://github.com/denoland/deno_core/pull/981


Changed files:
filename: Cargo.lock
patch: @@ -394,6 +394,12 @@ dependencies = [
  "tower-service",
 ]
 
+[[package]]
+name = "az"
+version = "1.2.1"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "7b7e4c2464d97fe331d41de9d5db0def0a96f4d823b8b32a2efd503578988973"
+
 [[package]]
 name = "backtrace"
 version = "0.3.73"
@@ -1454,11 +1460,12 @@ dependencies = [
 
 [[package]]
 name = "deno_core"
-version = "0.322.0"
+version = "0.323.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "2f593ef2b8acab8cd3ace9d50052edc65a3654fdbde808070cfa5da5cf7aaae6"
+checksum = "a781bcfe1b5211b8497f45bf5b3dba73036b8d5d1533c1f05d26ccf0afb25a78"
 dependencies = [
  "anyhow",
+ "az",
  "bincode",
  "bit-set",
  "bit-vec",
@@ -1969,9 +1976,9 @@ dependencies = [
 
 [[package]]
 name = "deno_ops"
-version = "0.198.0"
+version = "0.199.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "870826735cd9aa0376d2aadca14365b753e830e3cc16891efb9232845a6982a4"
+checksum = "a24a1f3e22029a57d3094b32070b8328eac793920b5a022027d360f085e6b245"
 dependencies = [
  "proc-macro-rules",
  "proc-macro2",
@@ -6570,9 +6577,9 @@ dependencies = [
 
 [[package]]
 name = "serde_v8"
-version = "0.231.0"
+version = "0.232.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "1a0c48b8842ebae21c52da1d978fba5c2be5991680bddfdc1a36ee0ccbc60114"
+checksum = "5c9feae92f7293fcc1a32a86be1a399859c0637e55dad8991d5258c43f7ff4d2"
 dependencies = [
  "num-bigint",
  "serde",

filename: Cargo.toml
patch: @@ -47,7 +47,7 @@ repository = "https://github.com/denoland/deno"
 
 [workspace.dependencies]
 deno_ast = { version = "=0.43.3", features = ["transpiling"] }
-deno_core = { version = "0.322.0" }
+deno_core = { version = "0.323.0" }
 
 deno_bench_util = { version = "0.173.0", path = "./bench_util" }
 deno_config = { version = "=0.39.2", features = ["workspace", "sync"] }

filename: ext/webstorage/01_webstorage.js
patch: @@ -3,91 +3,20 @@
 /// <reference path="../../core/internal.d.ts" />
 
 import { primordials } from "ext:core/mod.js";
-import {
-  op_webstorage_clear,
-  op_webstorage_get,
-  op_webstorage_iterate_keys,
-  op_webstorage_key,
-  op_webstorage_length,
-  op_webstorage_remove,
-  op_webstorage_set,
-} from "ext:core/ops";
+import { op_webstorage_iterate_keys, Storage } from "ext:core/ops";
 const {
-  Symbol,
   SymbolFor,
   ObjectFromEntries,
   ObjectEntries,
   ReflectDefineProperty,
   ReflectDeleteProperty,
-  ReflectGet,
+  FunctionPrototypeBind,
   ReflectHas,
   Proxy,
 } = primordials;
 
-import * as webidl from "ext:deno_webidl/00_webidl.js";
-
-const _persistent = Symbol("[[persistent]]");
-
-class Storage {
-  [_persistent];
-
-  constructor() {
-    webidl.illegalConstructor();
-  }
-
-  get length() {
-    webidl.assertBranded(this, StoragePrototype);
-    return op_webstorage_length(this[_persistent]);
-  }
-
-  key(index) {
-    webidl.assertBranded(this, StoragePrototype);
-    const prefix = "Failed to execute 'key' on 'Storage'";
-    webidl.requiredArguments(arguments.length, 1, prefix);
-    index = webidl.converters["unsigned long"](index, prefix, "Argument 1");
-
-    return op_webstorage_key(index, this[_persistent]);
-  }
-
-  setItem(key, value) {
-    webidl.assertBranded(this, StoragePrototype);
-    const prefix = "Failed to execute 'setItem' on 'Storage'";
-    webidl.requiredArguments(arguments.length, 2, prefix);
-    key = webidl.converters.DOMString(key, prefix, "Argument 1");
-    value = webidl.converters.DOMString(value, prefix, "Argument 2");
-
-    op_webstorage_set(key, value, this[_persistent]);
-  }
-
-  getItem(key) {
-    webidl.assertBranded(this, StoragePrototype);
-    const prefix = "Failed to execute 'getItem' on 'Storage'";
-    webidl.requiredArguments(arguments.length, 1, prefix);
-    key = webidl.converters.DOMString(key, prefix, "Argument 1");
-
-    return op_webstorage_get(key, this[_persistent]);
-  }
-
-  removeItem(key) {
-    webidl.assertBranded(this, StoragePrototype);
-    const prefix = "Failed to execute 'removeItem' on 'Storage'";
-    webidl.requiredArguments(arguments.length, 1, prefix);
-    key = webidl.converters.DOMString(key, prefix, "Argument 1");
-
-    op_webstorage_remove(key, this[_persistent]);
-  }
-
-  clear() {
-    webidl.assertBranded(this, StoragePrototype);
-    op_webstorage_clear(this[_persistent]);
-  }
-}
-
-const StoragePrototype = Storage.prototype;
-
 function createStorage(persistent) {
-  const storage = webidl.createBranded(Storage);
-  storage[_persistent] = persistent;
+  const storage = new Storage(persistent);
 
   const proxy = new Proxy(storage, {
     deleteProperty(target, key) {
@@ -106,12 +35,16 @@ function createStorage(persistent) {
       return true;
     },
 
-    get(target, key, receiver) {
+    get(target, key) {
       if (typeof key === "symbol") {
         return target[key];
       }
       if (ReflectHas(target, key)) {
-        return ReflectGet(target, key, receiver);
+        const value = target[key];
+        if (typeof value === "function") {
+          return FunctionPrototypeBind(value, target);
+        }
+        return value;
       }
       return target.getItem(key) ?? undefined;
     },
@@ -136,7 +69,7 @@ function createStorage(persistent) {
     },
 
     ownKeys() {
-      return op_webstorage_iterate_keys(persistent);
+      return op_webstorage_iterate_keys(storage);
     },
 
     getOwnPropertyDescriptor(target, key) {
@@ -163,7 +96,7 @@ function createStorage(persistent) {
     inspect,
     inspectOptions,
   ) {
-    return `${this.constructor.name} ${
+    return `Storage ${
       inspect({
         ...ObjectFromEntries(ObjectEntries(proxy)),
         length: this.length,

filename: ext/webstorage/lib.rs
patch: @@ -5,6 +5,7 @@
 use std::path::PathBuf;
 
 use deno_core::op2;
+use deno_core::GarbageCollected;
 use deno_core::OpState;
 use rusqlite::params;
 use rusqlite::Connection;
@@ -32,17 +33,14 @@ const MAX_STORAGE_BYTES: usize = 10 * 1024 * 1024;
 deno_core::extension!(deno_webstorage,
   deps = [ deno_webidl ],
   ops = [
-    op_webstorage_length,
-    op_webstorage_key,
-    op_webstorage_set,
-    op_webstorage_get,
-    op_webstorage_remove,
-    op_webstorage_clear,
     op_webstorage_iterate_keys,
   ],
+  objects = [
+    Storage
+  ],
   esm = [ "01_webstorage.js" ],
   options = {
-    origin_storage_dir: Option<PathBuf>
+      origin_storage_dir: Option<PathBuf>
   },
   state = |state, options| {
     if let Some(origin_storage_dir) = options.origin_storage_dir {
@@ -110,122 +108,134 @@ fn get_webstorage(
   Ok(conn)
 }
 
-#[op2(fast)]
-pub fn op_webstorage_length(
-  state: &mut OpState,
-  persistent: bool,
-) -> Result<u32, WebStorageError> {
-  let conn = get_webstorage(state, persistent)?;
-
-  let mut stmt = conn.prepare_cached("SELECT COUNT(*) FROM data")?;
-  let length: u32 = stmt.query_row(params![], |row| row.get(0))?;
+#[inline]
+fn size_check(input: usize) -> Result<(), WebStorageError> {
+  if input >= MAX_STORAGE_BYTES {
+    return Err(WebStorageError::StorageExceeded);
+  }
 
-  Ok(length)
+  Ok(())
 }
 
-#[op2]
-#[string]
-pub fn op_webstorage_key(
-  state: &mut OpState,
-  #[smi] index: u32,
+struct Storage {
   persistent: bool,
-) -> Result<Option<String>, WebStorageError> {
-  let conn = get_webstorage(state, persistent)?;
+}
 
-  let mut stmt =
-    conn.prepare_cached("SELECT key FROM data LIMIT 1 OFFSET ?")?;
+impl GarbageCollected for Storage {}
 
-  let key: Option<String> = stmt
-    .query_row(params![index], |row| row.get(0))
-    .optional()?;
+#[op2]
+impl Storage {
+  #[constructor]
+  #[cppgc]
+  fn new(persistent: bool) -> Storage {
+    Storage { persistent }
+  }
 
-  Ok(key)
-}
+  #[getter]
+  #[smi]
+  fn length(&self, state: &mut OpState) -> Result<u32, WebStorageError> {
+    let conn = get_webstorage(state, self.persistent)?;
 
-#[inline]
-fn size_check(input: usize) -> Result<(), WebStorageError> {
-  if input >= MAX_STORAGE_BYTES {
-    return Err(WebStorageError::StorageExceeded);
+    let mut stmt = conn.prepare_cached("SELECT COUNT(*) FROM data")?;
+    let length: u32 = stmt.query_row(params![], |row| row.get(0))?;
+
+    Ok(length)
   }
 
-  Ok(())
-}
+  #[required(1)]
+  #[string]
+  fn key(
+    &self,
+    state: &mut OpState,
+    #[smi] index: u32,
+  ) -> Result<Option<String>, WebStorageError> {
+    let conn = get_webstorage(state, self.persistent)?;
 
-#[op2(fast)]
-pub fn op_webstorage_set(
-  state: &mut OpState,
-  #[string] key: &str,
-  #[string] value: &str,
-  persistent: bool,
-) -> Result<(), WebStorageError> {
-  let conn = get_webstorage(state, persistent)?;
+    let mut stmt =
+      conn.prepare_cached("SELECT key FROM data LIMIT 1 OFFSET ?")?;
 
-  size_check(key.len() + value.len())?;
+    let key: Option<String> = stmt
+      .query_row(params![index], |row| row.get(0))
+      .optional()?;
 
-  let mut stmt = conn
-    .prepare_cached("SELECT SUM(pgsize) FROM dbstat WHERE name = 'data'")?;
-  let size: u32 = stmt.query_row(params![], |row| row.get(0))?;
+    Ok(key)
+  }
 
-  size_check(size as usize)?;
+  #[fast]
+  #[required(2)]
+  fn set_item(
+    &self,
+    state: &mut OpState,
+    #[string] key: &str,
+    #[string] value: &str,
+  ) -> Result<(), WebStorageError> {
+    let conn = get_webstorage(state, self.persistent)?;
 
-  let mut stmt = conn
-    .prepare_cached("INSERT OR REPLACE INTO data (key, value) VALUES (?, ?)")?;
-  stmt.execute(params![key, value])?;
+    size_check(key.len() + value.len())?;
 
-  Ok(())
-}
+    let mut stmt = conn
+      .prepare_cached("SELECT SUM(pgsize) FROM dbstat WHERE name = 'data'")?;
+    let size: u32 = stmt.query_row(params![], |row| row.get(0))?;
 
-#[op2]
-#[string]
-pub fn op_webstorage_get(
-  state: &mut OpState,
-  #[string] key_name: String,
-  persistent: bool,
-) -> Result<Option<String>, WebStorageError> {
-  let conn = get_webstorage(state, persistent)?;
+    size_check(size as usize)?;
 
-  let mut stmt = conn.prepare_cached("SELECT value FROM data WHERE key = ?")?;
-  let val = stmt
-    .query_row(params![key_name], |row| row.get(0))
-    .optional()?;
+    let mut stmt = conn.prepare_cached(
+      "INSERT OR REPLACE INTO data (key, value) VALUES (?, ?)",
+    )?;
+    stmt.execute(params![key, value])?;
 
-  Ok(val)
-}
+    Ok(())
+  }
 
-#[op2(fast)]
-pub fn op_webstorage_remove(
-  state: &mut OpState,
-  #[string] key_name: &str,
-  persistent: bool,
-) -> Result<(), WebStorageError> {
-  let conn = get_webstorage(state, persistent)?;
+  #[required(1)]
+  #[string]
+  fn get_item(
+    &self,
+    state: &mut OpState,
+    #[string] key: &str,
+  ) -> Result<Option<String>, WebStorageError> {
+    let conn = get_webstorage(state, self.persistent)?;
 
-  let mut stmt = conn.prepare_cached("DELETE FROM data WHERE key = ?")?;
-  stmt.execute(params![key_name])?;
+    let mut stmt =
+      conn.prepare_cached("SELECT value FROM data WHERE key = ?")?;
+    let val = stmt.query_row(params![key], |row| row.get(0)).optional()?;
 
-  Ok(())
-}
+    Ok(val)
+  }
 
-#[op2(fast)]
-pub fn op_webstorage_clear(
-  state: &mut OpState,
-  persistent: bool,
-) -> Result<(), WebStorageError> {
-  let conn = get_webstorage(state, persistent)?;
+  #[fast]
+  #[required(1)]
+  fn remove_item(
+    &self,
+    state: &mut OpState,
+    #[string] key: &str,
+  ) -> Result<(), WebStorageError> {
+    let conn = get_webstorage(state, self.persistent)?;
 
-  let mut stmt = conn.prepare_cached("DELETE FROM data")?;
-  stmt.execute(params![])?;
+    let mut stmt = conn.prepare_cached("DELETE FROM data WHERE key = ?")?;
+    stmt.execute(params![key])?;
 
-  Ok(())
+    Ok(())
+  }
+
+  #[fast]
+  fn clear(&self, state: &mut OpState) -> Result<(), WebStorageError> {
+    let conn = get_webstorage(state, self.persistent)?;
+
+    let mut stmt = conn.prepare_cached("DELETE FROM data")?;
+    stmt.execute(params![])?;
+
+    Ok(())
+  }
 }
 
 #[op2]
 #[serde]
-pub fn op_webstorage_iterate_keys(
+fn op_webstorage_iterate_keys(
+  #[cppgc] storage: &Storage,
   state: &mut OpState,
-  persistent: bool,
 ) -> Result<Vec<String>, WebStorageError> {
-  let conn = get_webstorage(state, persistent)?;
+  let conn = get_webstorage(state, storage.persistent)?;
 
   let mut stmt = conn.prepare_cached("SELECT key FROM data")?;
   let keys = stmt

filename: tests/wpt/runner/expectation.json
patch: @@ -9789,7 +9789,7 @@
     "event_constructor.window.html": false,
     "event_initstorageevent.window.html": false,
     "missing_arguments.window.html": true,
-    "storage_builtins.window.html": true,
+    "storage_builtins.window.html": false,
     "storage_clear.window.html": true,
     "storage_functions_not_overwritten.window.html": true,
     "storage_getitem.window.html": true,