use private symbols instead of private fields

For the version of V8 we're currently using, private symbol properties are about 10x faster than private fields. This may change in the future with optimizations in V8, but for now it's the faster approach.
IntrinsicLabs · Jul 27, 2019 · 5afdc5b · 5afdc5b
1 parent 5d6d247
commit 5afdc5b
Show file tree

Hide file tree

Showing 7 changed files with 95 additions and 52 deletions.
diff --git a/js/bootstrap/form_data.js b/js/bootstrap/form_data.js
@@ -1,14 +1,17 @@
+const dataSym = _bindings.getPrivate('data');
+
 export default class FormData {
 
   // TODO: This could be a Map<name, [[value, filename?]]> for efficient lookups
   // However, there shouldn't be more than a dozen entries
   // Duplicate names are allowed to exist otherwise it could be a simple Map<name, [value, filename?]>
-  #data = [];
+  //#data = [];  // using `dataSym`
 
   constructor(form) {
     if (form) {
       throw new TypeError("Osgood FormData doesn't support a form argument");
     }
+    this[dataSym] = [];
   }
 
   // FormData can have duplicate entries
@@ -25,7 +28,7 @@ export default class FormData {
     //   d.push(String(filename));
     // }
 
-    this.#data.push(d);
+    this[dataSym].push(d);
   }
 
   // destroys all existing entries with same name
@@ -38,18 +41,18 @@ export default class FormData {
   delete(name) {
     const new_data = [];
 
-    for (let entry of this.#data) {
+    for (let entry of this[dataSym]) {
       if (entry[0] !== name) {
         new_data.push(entry);
       }
     }
 
-    this.#data = new_data;
+    this[dataSym] = new_data;
   }
 
   // get first entry of `name`
   get(name) {
-    for (let entry of this.#data) {
+    for (let entry of this[dataSym]) {
       if (entry[0] === name) {
         return entry[1];
       }
@@ -60,7 +63,7 @@ export default class FormData {
   getAll(name) {
     const matches = [];
 
-    for (let entry of this.#data) {
+    for (let entry of this[dataSym]) {
       if (entry[0] === name) {
         matches.push(entry[1]);
       }
@@ -70,7 +73,7 @@ export default class FormData {
   }
 
   has(name) {
-    for (let entry of this.#data) {
+    for (let entry of this[dataSym]) {
       if (entry[0] === name) {
         return true
       }
@@ -80,30 +83,30 @@ export default class FormData {
   }
 
   entries() {
-    return this.#data[Symbol.iterator]();
+    return this[dataSym][Symbol.iterator]();
   }
 
   [Symbol.iterator]() {
-    return this.#data[Symbol.iterator]();
+    return this[dataSym][Symbol.iterator]();
   }
 
   // iterator<key>
   *keys() {
-    for (let entry of this.#data) {
+    for (let entry of this[dataSym]) {
       yield entry[0];
     }
   }
 
   // iterator<value>
   *values() {
-    for (let entry of this.#data) {
+    for (let entry of this[dataSym]) {
       yield entry[1];
     }
   }
 
   // not in the spec but Firefox and Chrome have it
   forEach(fn) {
-    for (let entry of this.#data) {
+    for (let entry of this[dataSym]) {
       fn(entry[0], entry[1], this);
     }
   }

diff --git a/js/bootstrap/request.js b/js/bootstrap/request.js
@@ -3,54 +3,63 @@ import { StringReadable, isBufferish } from 'internal:common.js';
 import Headers from 'internal:headers.js';
 import FormData from 'internal:form_data.js';
 
+const { getPrivate } = _bindings;
+
+const rawHeadersSym = getPrivate('rawHeaders');
+const headersSym = getPrivate('headers');
+const urlSym = getPrivate('url');
+const methodSym = getPrivate('method');
+const bodySym = getPrivate('body');
+const _bodyStringSym = getPrivate('_bodyString');
+
 export default class Request {
-  #rawHeaders; // not yet instantiated
-  #headers; // instantiated
-  #url;
-  #method;
-  #body;
-  #_bodyString;
+  // #rawHeaders; // not yet instantiated
+  // #headers; // instantiated
+  // #url;
+  // #method;
+  // #body;
+  // #_bodyString;
   constructor(input, init = {}) {
     // TODO support `input` being a Request
-    this.#url = input;
+    this[urlSym] = input;
 
     if (!(init.headers instanceof Headers)) {
-      this.#rawHeaders = init.headers;
+      this[rawHeadersSym] = init.headers;
     } else {
-      this.#headers = init.headers;
+      this[headersSym] = init.headers;
     }
-    this.#method = init.method || 'GET';
+    this[methodSym] = init.method || 'GET';
 
     if (init.body instanceof ReadableStream || init.body instanceof FormData) {
-      this.#body = init.body;
+      this[bodySym] = init.body;
     } else if (typeof init.body === 'string') {
-      this.#_bodyString = init.body;
+      this[_bodyStringSym] = init.body;
     } else if (isBufferish(init.body)) {
-      this.#body = new StringReadable(init.body);
+      this[bodySym] = new StringReadable(init.body);
     }
   }
 
   get headers() {
-    if (!this.#headers) {
-      this.#headers = new Headers(this.#rawHeaders);
+    if (!this[headersSym]) {
+      this[headersSym] = new Headers(this[rawHeadersSym]);
     }
-    return this.#headers;
+    return this[headersSym];
   }
 
   get url() {
-    return this.#url;
+    return this[urlSym];
   }
 
   get method() {
-    return this.#method;
+    return this[methodSym];
   }
 
   get body() {
-    return this.#body;
+    return this[bodySym];
   }
 
   get _bodyString() {
-    return this.#_bodyString;
+    return this[_bodyStringSym];
   }
 }
 BodyMixin.mixin(Request);
diff --git a/js/bootstrap/response.js b/js/bootstrap/response.js
@@ -2,51 +2,60 @@ import BodyMixin from 'internal:body_mixin.js';
 import { StringReadable, isBufferish } from 'internal:common.js';
 import Headers from 'internal:headers.js';
 
+const { getPrivate } = _bindings;
+
+const rawHeadersSym = getPrivate('rawHeaders');
+const headersSym = getPrivate('headers');
+const statusSym = getPrivate('status');
+const statusTextSym = getPrivate('statusText');
+const bodySym = getPrivate('body');
+const _bodyStringSym = getPrivate('_bodyString');
+
 export default class Response {
-  #rawHeaders; // not yet instantiated
-  #headers; // instantiated
-  #status;
-  #statusText;
-  #body;
-  #_bodyString;
+  // #rawHeaders; // not yet instantiated
+  // #headers; // instantiated
+  // #status;
+  // #statusText;
+  // #body;
+  // #_bodyString;
   constructor(body, init = {}) {
-    this.#status = init.status || 200;
-    this.#statusText = init.statusText || 'OK';
+    this[statusSym] = init.status || 200;
+    this[statusTextSym] = init.statusText || 'OK';
     if (!(init.headers instanceof Headers)) {
-      this.#rawHeaders = init.headers;
+      this[rawHeadersSym] = init.headers;
     } else {
-      this.#headers = init.headers;
+      this[headersSym] = init.headers;
     }
     if (body instanceof ReadableStream || body instanceof TransformStream) {
-      this.#body = body;
+      this[bodySym] = body;
     } else if (typeof body === 'string') {
-      this.#_bodyString = body;
+      this[_bodyStringSym] = body;
     } else if (isBufferish(body)) {
-      this.#body = new StringReadable(body);
+      this[bodySym] = new StringReadable(body);
     }
   }
 
   get headers() {
-    if (!this.#headers) {
-      this.#headers = new Headers(this.#rawHeaders);
+    if (!this[headersSym]) {
+      this[headersSym] = new Headers(this[rawHeadersSym]);
     }
-    return this.#headers;
+    return this[headersSym];
   }
 
   get status() {
-    return this.#status;
+    return this[statusSym];
   }
 
   get statusText() {
-    return this.#statusText;
+    return this[statusTextSym];
   }
 
   get body() {
-    return this.#body;
+    return this[bodySym];
   }
 
   get _bodyString() {
-    return this.#_bodyString;
+    return this[_bodyStringSym];
   }
 
 }

diff --git a/osgood-v8/src/wrapper/local.rs b/osgood-v8/src/wrapper/local.rs
@@ -198,6 +198,7 @@ each_valuable_type!(V8::Function);
 each_valuable_type!(V8::Number);
 each_valuable_type!(V8::Integer);
 each_valuable_type!(V8::ArrayBuffer);
+each_valuable_type!(V8::Private);
 
 // For V8::Value, this is not done with each_valuable_type! because the From is already implemented
 // for identical types

diff --git a/osgood-v8/src/wrapper/mod.rs b/osgood-v8/src/wrapper/mod.rs
@@ -48,6 +48,9 @@ pub use array_buffer::*;
 mod exception;
 pub use exception::*;
 
+mod private;
+pub use private::*;
+
 /// This is a convenience `None`, which can be used by reference as a "null" in arguments to v8
 /// functions.
 pub const NULL: Option<u16> = None;

diff --git a/osgood-v8/src/wrapper/private.rs b/osgood-v8/src/wrapper/private.rs
@@ -0,0 +1,11 @@
+use super::*;
+
+pub use V8::Private;
+
+impl Private {
+    pub fn for_api(name: &str) -> Local<Private> {
+        unsafe {
+            V8::Private_ForApi(Isolate::raw(), V8::String::new_from_slice(name).into()).into()
+        }
+    }
+}
diff --git a/src/worker.rs b/src/worker.rs
@@ -242,6 +242,7 @@ fn make_globals(mut context: Local<Context>, route: &str) {
     if let Ok(_var) = std::env::var("DEBUG") {
         obj.set_extern_method(context, "debug", debug);
     }
+    obj.set_extern_method(context, "getPrivate", get_private);
     global.set("_bindings", obj);
 }
 
@@ -349,3 +350,9 @@ fn error(args: FunctionCallbackInfo) {
 fn debug(args: FunctionCallbackInfo) {
     log_debug!("JS debug: {}", args.get(0).unwrap().as_rust_string());
 }
+
+#[v8_fn]
+fn get_private(args: FunctionCallbackInfo) {
+    let ret = Private::for_api(&args.get(0).unwrap().as_rust_string());
+    args.set_return_value(&ret);
+}