Skip to content

Conversation

@ExE-Boss
Copy link
Contributor

Currently, a new Proxy handler is created per‑instance, this is inefficient and unnecessary, especially since the proxy handlers don’t depend on any instance‑specific state.

@domenic
Copy link
Member

domenic commented Jan 16, 2020

Can you show before/after benchmarks to demonstrate a perf improvement?

@TimothyGu
Copy link
Member

This looks good but I'd also be interested in seeing benchmark results, especially in terms of memory usage. I.e., how much memory does an additional handler with its own copy of handler methods actually take?

@ExE-Boss
Copy link
Contributor Author

ExE-Boss commented Jan 18, 2020

There’s a decent performance increase with this:

benchmark per‑instance Proxy handler per‑class Proxy handler
dom/compare-document-position/compare ancestor jsdom × 33,877 ops/sec ±2.88% (14 runs sampled) jsdom × 35,270 ops/sec ±2.83% (15 runs sampled)
dom/compare-document-position/compare descendant jsdom × 34,320 ops/sec ±2.22% (14 runs sampled) jsdom × 34,349 ops/sec ±4.54% (14 runs sampled)
dom/compare-document-position/compare siblings jsdom × 1,316,711 ops/sec ±3.36% (9 runs sampled) jsdom × 1,407,317 ops/sec ±1.92% (12 runs sampled)
dom/construction/createComment jsdom × 1,235,394 ops/sec ±4.44% (92 runs sampled) jsdom × 1,248,180 ops/sec ±2.31% (89 runs sampled)
dom/construction/createDocumentFragment jsdom × 1,270,944 ops/sec ±0.41% (97 runs sampled) jsdom × 1,288,443 ops/sec ±0.44% (98 runs sampled)
dom/construction/createElement jsdom × 221,507 ops/sec ±2.86% (87 runs sampled) jsdom × 324,510 ops/sec ±0.39% (98 runs sampled)
dom/construction/createEvent jsdom × 191,764 ops/sec ±1.94% (92 runs sampled) jsdom × 193,381 ops/sec ±1.78% (89 runs sampled)
dom/construction/createNodeIterator jsdom × 1,247,649 ops/sec ±0.28% (94 runs sampled) jsdom × 1,274,109 ops/sec ±0.41% (98 runs sampled)
dom/construction/createProcessingInstruction jsdom × 627,401 ops/sec ±0.32% (95 runs sampled) jsdom × 633,693 ops/sec ±0.38% (95 runs sampled)
dom/construction/createTextNode jsdom × 973,704 ops/sec ±0.49% (99 runs sampled) jsdom × 999,581 ops/sec ±2.17% (90 runs sampled)
dom/inner-html/tables jsdom × 0.33 ops/sec ±1.98% (5 runs sampled) jsdom × 0.36 ops/sec ±3.74% (5 runs sampled)
dom/named-properties/setAttribute(): Remove a named property from window jsdom × 2,904 ops/sec ±4.53% (56 runs sampled) jsdom × 3,488 ops/sec ±0.94% (61 runs sampled)
dom/tree-modification/appendChild: many parents jsdom × 28.77 ops/sec ±0.84% (49 runs sampled) jsdom × 30.67 ops/sec ±0.80% (52 runs sampled)
dom/tree-modification/appendChild: many siblings jsdom × 245,236 ops/sec ±13.16% (16 runs sampled) jsdom × 270,620 ops/sec ±10.80% (17 runs sampled)
dom/tree-modification/appendChild: no siblings jsdom × 217,962 ops/sec ±11.74% (21 runs sampled) jsdom × 261,350 ops/sec ±6.80% (25 runs sampled)
dom/tree-modification/insertBefore: many siblings jsdom × 236,425 ops/sec ±12.12% (15 runs sampled) jsdom × 269,569 ops/sec ±1.72% (18 runs sampled)
dom/tree-modification/removeChild: many parents jsdom × 122 ops/sec ±1.07% (64 runs sampled) jsdom × 130 ops/sec ±0.77% (63 runs sampled)
dom/tree-modification/removeChild: many siblings jsdom × 133,626 ops/sec ±7.42% (35 runs sampled) jsdom × 149,070 ops/sec ±7.08% (38 runs sampled)
dom/tree-modification/removeChild: no siblings jsdom × 118,199 ops/sec ±11.44% (23 runs sampled) jsdom × 136,483 ops/sec ±10.06% (30 runs sampled)
html/parsing/text jsdom × 14.23 ops/sec ±1.51% (40 runs sampled) jsdom × 14.34 ops/sec ±1.82% (41 runs sampled)
jsdom/api/new JSDOM() defaults <Test #⁠10> × 261 ops/sec ±4.49% (73 runs sampled) <Test #⁠10> × 303 ops/sec ±4.44% (77 runs sampled)
jsdom/api/new JSDOM() with many elements <Test #⁠11> × 27.18 ops/sec ±14.82% (51 runs sampled) <Test #⁠11> × 33.18 ops/sec ±4.74% (60 runs sampled)

@ExE-Boss ExE-Boss changed the title perf(proxy): Use shared Proxy handler perf(proxy): Use per‑class Proxy handler Jan 18, 2020
@domenic
Copy link
Member

domenic commented Jan 18, 2020

Nice. In that case let's merge this.

@domenic domenic merged commit 7c8037f into jsdom:master Jan 18, 2020
@domenic domenic deleted the perf/proxy/use-shared-proxy-handler branch January 18, 2020 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants