feat: Use WeakMap for wrapper to impl conversion

[ExE-Boss]

Currently, wrapper → impl mapping is done using a symbol.

The issue with this approach is that it causes several observable [[Get]]s on the object, which causes some WPT failures, and can lead to implementation leakage, as described in the class fields proposal.

[domenic]

Thanks; I've been interested in doing this for some time. But, I'm concerned about the performance hit. Can you post the results of the jsdom benchmark suite before and after?

[ExE-Boss]

Well, here’s the benchmark result for Node.js 13.5.0 on my PC:

benchmark	symbol	WeakMap
dom/compare-document-position/compare ancestor	jsdom × 33,877 ops/sec ±2.88% (14 runs sampled)	jsdom × 30,475 ops/sec ±3.29% (11 runs sampled)
dom/compare-document-position/compare descendant	jsdom × 34,320 ops/sec ±2.22% (14 runs sampled)	jsdom × 24,739 ops/sec ±41.56% (11 runs sampled)
dom/compare-document-position/compare siblings	jsdom × 1,316,711 ops/sec ±3.36% (9 runs sampled)	jsdom × 152,480 ops/sec ±196.14% (6 runs sampled)
dom/construction/createComment	jsdom × 1,235,394 ops/sec ±4.44% (92 runs sampled)	jsdom × 45,629 ops/sec ±60.45% (60 runs sampled)
dom/construction/createDocumentFragment	jsdom × 1,270,944 ops/sec ±0.41% (97 runs sampled)	jsdom × 295,794 ops/sec ±29.02% (49 runs sampled)
dom/construction/createElement	jsdom × 221,507 ops/sec ±2.86% (87 runs sampled)	jsdom × 74,368 ops/sec ±31.28% (73 runs sampled)
dom/construction/createEvent	jsdom × 191,764 ops/sec ±1.94% (92 runs sampled)	jsdom × 150,126 ops/sec ±1.37% (93 runs sampled)
dom/construction/createNodeIterator	jsdom × 1,247,649 ops/sec ±0.28% (94 runs sampled)	jsdom × 101,389 ops/sec ±52.98% (17 runs sampled)
dom/construction/createProcessingInstruction	jsdom × 627,401 ops/sec ±0.32% (95 runs sampled)	jsdom × 110,947 ops/sec ±41.93% (74 runs sampled)
dom/construction/createTextNode	jsdom × 973,704 ops/sec ±0.49% (99 runs sampled)	jsdom × 320,028 ops/sec ±22.86% (79 runs sampled)
dom/inner-html/tables	jsdom × 0.33 ops/sec ±1.98% (5 runs sampled)	jsdom × 0.28 ops/sec ±1.72% (5 runs sampled)
dom/named-properties/setAttribute(): Remove a named property from window	jsdom × 2,904 ops/sec ±4.53% (56 runs sampled)	jsdom × 1,596 ops/sec ±42.04% (29 runs sampled)
dom/tree-modification/appendChild: many parents	jsdom × 28.77 ops/sec ±0.84% (49 runs sampled)	jsdom × 27.18 ops/sec ±0.85% (45 runs sampled)
dom/tree-modification/appendChild: many siblings	jsdom × 245,236 ops/sec ±13.16% (16 runs sampled)	jsdom × 83,316 ops/sec ±80.33% (8 runs sampled)
dom/tree-modification/appendChild: no siblings	jsdom × 217,962 ops/sec ±11.74% (21 runs sampled)	jsdom × 45,244 ops/sec ±69.13% (8 runs sampled)
dom/tree-modification/insertBefore: many siblings	jsdom × 236,425 ops/sec ±12.12% (15 runs sampled)	jsdom × 42,729 ops/sec ±146.72% (5 runs sampled)
dom/tree-modification/removeChild: many parents	jsdom × 122 ops/sec ±1.07% (64 runs sampled)	jsdom × 121 ops/sec ±0.99% (61 runs sampled)
dom/tree-modification/removeChild: many siblings	jsdom × 133,626 ops/sec ±7.42% (35 runs sampled)	jsdom × 36,757 ops/sec ±33.78% (9 runs sampled)
dom/tree-modification/removeChild: no siblings	jsdom × 118,199 ops/sec ±11.44% (23 runs sampled)	jsdom × 20,775 ops/sec ±4.47% (15 runs sampled)
html/parsing/text	jsdom × 14.23 ops/sec ±1.51% (40 runs sampled)	jsdom × 13.11 ops/sec ±28.91% (43 runs sampled)
jsdom/api/new JSDOM() defaults	<Test #⁠10> × 261 ops/sec ±4.49% (73 runs sampled)	<Test #⁠10> × 255 ops/sec ±15.80% (74 runs sampled)
jsdom/api/new JSDOM() with many elements	<Test #⁠11> × 27.18 ops/sec ±14.82% (51 runs sampled)	<Test #⁠11> × 18.54 ops/sec ±21.99% (38 runs sampled)

[TimothyGu]

Could you also benchmark running esmarkup over the ECMA-262 source with/without this patch? (See command for doing so.)

[Sebmaster]

The large variability combined with the giant slowdown in the create* tests makes me kinda wary of this. Are we doing something weird in createNode that taxes on WeakMaps so much?

[pmdartus]

I ran the benchmark locally and I was able to measure a noticeable slowdown locally. All the interface creation tests are 3x slower using WeakMap instead of a Symbol. You can see below the benchmark results I gathered locally on Node 12.13.1. I also ran the same benchmark against Node 13.6.0, and the results are comparable.

The variability of the benchmark results might be explained by 2 things:

• with the overall slowdown, benchmark.js uses fewer samples for each benchmark.
• looking at a couple of CPU profiles, we can see that each GC takes way longer in the WeakMap version compared to the Symbol one.

Based on the benchmark data, I feel uncomfortable introducing such a large regression compared to the value-added.

---

Benchmark result on Macbook Pro 2015 / Node 12.13.1

Test name	Symbol mapping	WeakMap mapping
dom/compare-document-position/compare ancestor	18,624 ops/sec ±12.65%	18,319 ops/sec ±2.45%
dom/compare-document-position/compare descendant	21,323 ops/sec ±6.25%	16,507 ops/sec ±6.04%
dom/compare-document-position/compare siblings	1,002,885 ops/sec ±26.70%	503,312 ops/sec ±8.50%
dom/construction/createComment	1,290,169 ops/sec ±1.27%	263,351 ops/sec ±26.75%
dom/construction/createDocumentFragment	1,377,951 ops/sec ±0.63%	307,647 ops/sec ±20.64%
dom/construction/createElement	193,831 ops/sec ±5.56%	43,275 ops/sec ±38.74%
dom/construction/createEvent	212,986 ops/sec ±4.40%	25,691 ops/sec ±21.27%
dom/construction/createNodeIterator	1,156,065 ops/sec ±0.44%	317,913 ops/sec ±26.28%
dom/construction/createProcessingInstruction	540,759 ops/sec ±0.93%	33,108 ops/sec ±3.30%
dom/construction/createTextNode	962,896 ops/sec ±0.66%	32,521 ops/sec ±3.05%
dom/inner-html/tables	0.31 ops/sec ±3.88%	0.11 ops/sec ±96.57%
dom/named-properties/setAttribute	3,124 ops/sec ±12.53%	2,523 ops/sec ±1.85%
dom/tree-modification/appendChild: many parents	21.95 ops/sec ±0.93%	20.36 ops/sec ±1.89%
dom/tree-modification/appendChild: many siblings	254,109 ops/sec ±17.94%	35,003 ops/sec ±140.39%
dom/tree-modification/appendChild: no siblings	238,119 ops/sec ±19.03%	36,006 ops/sec ±105.56%
dom/tree-modification/insertBefore: many siblings	194,787 ops/sec ±22.13%	29,815 ops/sec ±89.69%
dom/tree-modification/removeChild: many parents	113 ops/sec ±16.32%	74.89 ops/sec ±1.28%
dom/tree-modification/removeChild: many siblings	96,538 ops/sec ±12.43%	21,796 ops/sec ±23.13%
dom/tree-modification/removeChild: no siblings	79,519 ops/sec ±24.92%	26,069 ops/sec ±43.54%
html/parsing/text	13.26 ops/sec ±1.86%	0.76 ops/sec ±261.72%
jsdom/api/new JSDOM() defaults	212 ops/sec ±19.43%	206 ops/sec ±27.32%
jsdom/api/new JSDOM() with many elements	18.81 ops/sec ±22.14%	15.18 ops/sec ±26.72%

[domenic]

I agree; this is not worth the cost. So, let's close.