Merge pull request #184 from b-fuze/memory-perf-optimizations

Memory perf optimizations

.github/workflows/main.yml

@@ -3,17 +3,17 @@ name: ci
 on:
   push:
     paths:
-      - 'Cargo.toml'
-      - 'html-parser/core/**'
-      - 'html-parser/plugin/**'
-      - '.github/workflows/**'
+      - "Cargo.toml"
+      - "html-parser/core/**"
+      - "html-parser/plugin/**"
+      - ".github/workflows/**"
   pull_request:
     branches-ignore: [master]
     paths:
-      - 'Cargo.toml'
-      - 'html-parser/core/**'
-      - 'html-parser/plugin/**'
-      - '.github/workflows/**'
+      - "Cargo.toml"
+      - "html-parser/core/**"
+      - "html-parser/plugin/**"
+      - ".github/workflows/**"
 
 jobs:
   build:
@@ -36,7 +36,7 @@ jobs:
       - name: Install rust
         uses: hecrj/setup-rust-action@v1
         with:
-          rust-version: '1.60.0'
+          rust-version: "1.60.0"
 
       - name: Log versions
         run: |
@@ -143,7 +143,7 @@ jobs:
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
         with:
-          tag_name: 'release draft'
+          tag_name: "release draft"
           draft: true
           files: |
             target/release/libplugin.dylib

README.md

@@ -139,6 +139,37 @@ To use the new binary you need to set the **`DENO_DOM_PLUGIN`** env var to the
 path of the binary produced in the previous step. **Don't forget** to run Deno
 with `--allow-env`.
 
+## Inconsistencies with standard APIs
+
+### Differences in `DOMTokenList`/`Element.classList`
+
+To optimize memory usage, Deno DOM doesn't allocate `DOMTokenList`s
+(`Element.classList`) on `Element` creation, it instead creates an
+`UninitializedDOMTokenList` object. This can cause a subtle deviation from
+standard APIs:
+
+```typescript
+const div = doc.createElement("div");
+
+// Retrieve the uninitialized DOMTokenList
+const { classList } = div;
+
+// Initialize the DOMTokenList by adding a few classes
+classList.add("foo");
+classList.add("bar");
+
+// Inconsistency: the uninitialized classList/DOMTokenList
+// is now a different object from the initialized one
+
+classList !== div.classList;
+
+// However, the uninitialized DOMTokenList object still
+// works as the initialized DOMTokenList object:
+
+classList.add("fizz");
+div.classList.contains("fizz") === true;
+```
+
 # Credits
 
 - html5ever developers for the HTML parser

deno.jsonc

@@ -35,6 +35,7 @@
     "exclude": [
       "./wpt",
       "./bench/node_modules",
+      "./bench/c.html",
       "./src/dom/selectors/nwsapi.js",
       "./src/dom/selectors/sizzle.js",
       "./target",

src/deserialize.ts

@@ -56,13 +56,13 @@ function nodeFromArray(data: any, parentNode: Node | null): Node {
     switch (child[0]) {
       case NodeType.TEXT_NODE:
         childNode = new Text(child[1]);
-        childNode.parentNode = childNode.parentElement = <Element> elm;
+        childNode.parentNode = <Element> elm;
         childNodes.push(childNode);
         break;
 
       case NodeType.COMMENT_NODE:
         childNode = new Comment(child[1]);
-        childNode.parentNode = childNode.parentElement = <Element> elm;
+        childNode.parentNode = <Element> elm;
         childNodes.push(childNode);
         break;
 
@@ -73,7 +73,7 @@ function nodeFromArray(data: any, parentNode: Node | null): Node {
 
       case NodeType.DOCUMENT_TYPE_NODE:
         childNode = new DocumentType(child[1], child[2], child[3], CTOR_KEY);
-        childNode.parentNode = childNode.parentElement = <Element> elm;
+        childNode.parentNode = <Element> elm;
         childNodes.push(childNode);
         break;
     }

src/dom/document-fragment.ts

@@ -137,7 +137,7 @@ function documentFragmentGetElementsByClassName(
   this: DocumentFragment,
   className: string,
 ) {
-  return getElementsByClassName(this, className, []);
+  return getElementsByClassName(this, className.trim().split(/\s+/), []);
 }
 
 function documentFragmentGetElementsByTagNameWildcard(

src/dom/document.ts

@@ -7,6 +7,7 @@ import { HTMLTemplateElement } from "./elements/html-template-element.ts";
 import { getSelectorEngine, SelectorApi } from "./selectors/selectors.ts";
 import { getElementsByClassName } from "./utils.ts";
 import UtilTypes from "./utils-types.ts";
+import { getUpperCase } from "./string-cache.ts";
 
 export class DOMImplementation {
   constructor(key: typeof CTOR_KEY) {
@@ -94,7 +95,7 @@ export class DocumentType extends Node {
     return this.#systemId;
   }
 
-  _shallowClone(): Node {
+  override _shallowClone(): Node {
     return new DocumentType(
       this.#qualifiedName,
       this.#publicId,
@@ -119,9 +120,7 @@ export class Document extends Node {
   public body: Element = <Element> <unknown> null;
   public implementation: DOMImplementation;
 
-  #lockState = false;
   #documentURI = "about:blank"; // TODO
-  #title = "";
   #nwapi: SelectorApi | null = null;
 
   constructor() {
@@ -135,7 +134,7 @@ export class Document extends Node {
     this.implementation = new DOMImplementation(CTOR_KEY);
   }
 
-  _shallowClone(): Node {
+  override _shallowClone(): Node {
     return new Document();
   }
 
@@ -215,14 +214,14 @@ export class Document extends Node {
     return count;
   }
 
-  appendChild(child: Node): Node {
+  override appendChild(child: Node): Node {
     super.appendChild(child);
     child._setOwnerDocument(this);
     return child;
   }
 
   createElement(tagName: string, options?: ElementCreationOptions): Element {
-    tagName = tagName.toUpperCase();
+    tagName = getUpperCase(tagName);
 
     switch (tagName) {
       case "TEMPLATE": {
@@ -299,7 +298,7 @@ export class Document extends Node {
   // but that would be a breaking change since `.body`
   // and `.head` would need to be typed as `Element | null`.
   // Currently they're typed as `Element` which is incorrect...
-  cloneNode(deep?: boolean): Document {
+  override cloneNode(deep?: boolean): Document {
     const doc = super.cloneNode(deep) as Document;
 
     for (const child of doc.documentElement?.childNodes || []) {
@@ -338,6 +337,10 @@ export class Document extends Node {
 
   // TODO: DRY!!!
   getElementById(id: string): Element | null {
+    if (!this._hasInitializedChildNodes()) {
+      return null;
+    }
+
     for (const child of this.childNodes) {
       if (child.nodeType === NodeType.ELEMENT_NODE) {
         if ((<Element> child).id === id) {
@@ -363,7 +366,7 @@ export class Document extends Node {
         )
         : [];
     } else {
-      return <Element[]> this._getElementsByTagName(tagName.toUpperCase(), []);
+      return <Element[]> this._getElementsByTagName(getUpperCase(tagName), []);
     }
   }
 
@@ -397,7 +400,11 @@ export class Document extends Node {
   }
 
   getElementsByClassName(className: string): Element[] {
-    return <Element[]> getElementsByClassName(this, className, []);
+    return getElementsByClassName(
+      this,
+      className.trim().split(/\s+/),
+      [],
+    ) as Element[];
   }
 
   hasFocus(): boolean {
@@ -413,7 +420,7 @@ export class HTMLDocument extends Document {
     super();
   }
 
-  _shallowClone(): Node {
+  override _shallowClone(): Node {
     return new HTMLDocument(CTOR_KEY);
   }
 }