[lit-html] Re-work async directives for better gc behavior #2044

kevinpschaaf · 2021-08-10T01:27:05Z

Re-works the disconnection handling for first-party AsyncDirectives (until, asyncAppend, asyncReplace) to use a WeakMap to connect an awaited promise's resolver and the directive that is awaiting it, so that a disconnected directive (and all of the DOM it references via its part) is not prevented from being GC'ed until its awaited promise resolves.

This effectively means they will have the previous behavior of deferring their commit until they are re-connected, but with the benefit of not leaking their DOM while disconnected.

Follow-on to #2034.

changeset-bot · 2021-08-10T01:27:07Z

🦋 Changeset detected

Latest commit: 0b6386b

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2021-08-10T01:27:26Z

📊 Tachometer Benchmark Results

Summary

nop-update

lit-html-kitchen-sink: unsure 🔍 -2% - +6% (-0.86ms - +2.05ms)
^{this-change vs tip-of-tree}

render

lit-element-list: unsure 🔍 -3% - +3% (-3.37ms - +2.83ms)
^{this-change vs tip-of-tree}
lit-html-kitchen-sink: unsure 🔍 -2% - +5% (-0.98ms - +1.99ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -8% - +3% (-1.16ms - +0.39ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -6% - +1% (-4.56ms - +0.53ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -2% - +2% (-1.21ms - +1.67ms)
^{this-change vs tip-of-tree}

update

lit-element-list: unsure 🔍 -2% - +2% (-15.52ms - +15.62ms)
^{this-change vs tip-of-tree}
lit-html-kitchen-sink: unsure 🔍 -4% - +4% (-4.54ms - +4.65ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -3% - +2% (-11.77ms - +10.03ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -3% - +3% (-3.68ms - +3.67ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -1% - +2% (-7.63ms - +19.71ms)
^{this-change vs tip-of-tree}

update-reflect

lit-element-list: unsure 🔍 -2% - +2% (-14.15ms - +18.09ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -1% - +2% (-10.24ms - +20.94ms)
^{this-change vs tip-of-tree}

Results

lit-element-list

Browser: chrome-headless 92.0.4515.131
Sample size: 50
Built by: Benchmarks #1686
Commit: dfb5adc

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	104.71ms - 109.27ms	-	unsure 🔍 -3% - +3% -3.37ms - +2.83ms	faster ✔ 23% - 28% 32.20ms - 41.62ms
tip-of-tree tip-of-tree	105.15ms - 109.37ms	unsure 🔍 -3% - +3% -2.83ms - +3.37ms	-	faster ✔ 23% - 28% 32.00ms - 41.27ms
previous-release previous-release	139.77ms - 148.02ms	slower ❌ 30% - 39% 32.20ms - 41.62ms	slower ❌ 29% - 39% 32.00ms - 41.27ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	874.89ms - 896.48ms	-	unsure 🔍 -2% - +2% -15.52ms - +15.62ms	faster ✔ 7% - 10% 62.27ms - 98.25ms
tip-of-tree tip-of-tree	874.42ms - 896.85ms	unsure 🔍 -2% - +2% -15.62ms - +15.52ms	-	faster ✔ 7% - 10% 62.06ms - 98.56ms
previous-release previous-release	951.55ms - 980.34ms	slower ❌ 7% - 11% 62.27ms - 98.25ms	slower ❌ 7% - 11% 62.06ms - 98.56ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	886.55ms - 908.15ms	-	unsure 🔍 -2% - +2% -14.15ms - +18.09ms	faster ✔ 4% - 8% 42.07ms - 74.28ms
tip-of-tree tip-of-tree	883.41ms - 907.36ms	unsure 🔍 -2% - +2% -18.09ms - +14.15ms	-	faster ✔ 5% - 8% 43.23ms - 77.06ms
previous-release previous-release	943.58ms - 967.47ms	slower ❌ 5% - 8% 42.07ms - 74.28ms	slower ❌ 5% - 9% 43.23ms - 77.06ms	-

lit-html-kitchen-sink

Browser: chrome-headless 92.0.4515.131
Sample size: 30
Built by: Benchmarks #1686
Commit: dfb5adc

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	42.87ms - 44.68ms	-	unsure 🔍 -2% - +5% -0.98ms - +1.99ms	faster ✔ 14% - 19% 7.29ms - 9.80ms
tip-of-tree tip-of-tree	42.09ms - 44.44ms	unsure 🔍 -5% - +2% -1.99ms - +0.98ms	-	faster ✔ 15% - 20% 7.59ms - 10.51ms
previous-release previous-release	51.45ms - 53.19ms	slower ❌ 16% - 23% 7.29ms - 9.80ms	slower ❌ 17% - 25% 7.59ms - 10.51ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	107.53ms - 113.20ms	-	unsure 🔍 -4% - +4% -4.54ms - +4.65ms	unsure 🔍 -6% - +1% -6.84ms - +0.87ms
tip-of-tree tip-of-tree	106.70ms - 113.92ms	unsure 🔍 -4% - +4% -4.65ms - +4.54ms	-	unsure 🔍 -7% - +1% -7.50ms - +1.42ms
previous-release previous-release	110.74ms - 115.96ms	unsure 🔍 -1% - +6% -0.87ms - +6.84ms	unsure 🔍 -1% - +7% -1.42ms - +7.50ms	-

nop-update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	36.95ms - 38.91ms	-	unsure 🔍 -2% - +6% -0.86ms - +2.05ms	unsure 🔍 -2% - +6% -0.74ms - +2.19ms
tip-of-tree tip-of-tree	36.27ms - 38.41ms	unsure 🔍 -5% - +2% -2.05ms - +0.86ms	-	unsure 🔍 -4% - +4% -1.39ms - +1.65ms
previous-release previous-release	36.12ms - 38.29ms	unsure 🔍 -6% - +2% -2.19ms - +0.74ms	unsure 🔍 -4% - +4% -1.65ms - +1.39ms	-

lit-html-repeat

Browser: chrome-headless 92.0.4515.131
Sample size: 30
Built by: Benchmarks #1686
Commit: dfb5adc

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	13.28ms - 14.13ms	-	unsure 🔍 -8% - +3% -1.16ms - +0.39ms	faster ✔ 12% - 19% 1.95ms - 3.20ms
tip-of-tree tip-of-tree	13.44ms - 14.74ms	unsure 🔍 -3% - +9% -0.39ms - +1.16ms	-	faster ✔ 9% - 18% 1.39ms - 2.99ms
previous-release previous-release	15.82ms - 16.74ms	slower ❌ 14% - 24% 1.95ms - 3.20ms	slower ❌ 9% - 22% 1.39ms - 2.99ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	397.11ms - 411.88ms	-	unsure 🔍 -3% - +2% -11.77ms - +10.03ms	faster ✔ 27% - 31% 151.91ms - 178.39ms
tip-of-tree tip-of-tree	397.35ms - 413.38ms	unsure 🔍 -2% - +3% -10.03ms - +11.77ms	-	faster ✔ 27% - 31% 150.68ms - 177.88ms
previous-release previous-release	558.66ms - 580.63ms	slower ❌ 37% - 45% 151.91ms - 178.39ms	slower ❌ 37% - 44% 150.68ms - 177.88ms	-

lit-html-template-heavy

Browser: chrome-headless 92.0.4515.131
Sample size: 50
Built by: Benchmarks #1686
Commit: dfb5adc

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	67.94ms - 70.62ms	-	unsure 🔍 -6% - +1% -4.56ms - +0.53ms	faster ✔ 15% - 19% 12.05ms - 15.62ms
tip-of-tree tip-of-tree	69.13ms - 73.46ms	unsure 🔍 -1% - +7% -0.53ms - +4.56ms	-	faster ✔ 11% - 17% 9.36ms - 14.29ms
previous-release previous-release	81.94ms - 84.29ms	slower ❌ 17% - 23% 12.05ms - 15.62ms	slower ❌ 13% - 20% 9.36ms - 14.29ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	137.66ms - 143.12ms	-	unsure 🔍 -3% - +3% -3.68ms - +3.67ms	faster ✔ 10% - 14% 15.33ms - 22.94ms
tip-of-tree tip-of-tree	137.94ms - 142.85ms	unsure 🔍 -3% - +3% -3.67ms - +3.68ms	-	faster ✔ 10% - 14% 15.52ms - 22.74ms
previous-release previous-release	156.88ms - 162.18ms	slower ❌ 11% - 17% 15.33ms - 22.94ms	slower ❌ 11% - 16% 15.52ms - 22.74ms	-

reactive-element-list

Browser: chrome-headless 92.0.4515.131
Sample size: 50
Built by: Benchmarks #1686
Commit: dfb5adc

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	72.54ms - 74.67ms	-	unsure 🔍 -2% - +2% -1.21ms - +1.67ms	unsure 🔍 -0% - +3% -0.30ms - +2.45ms
tip-of-tree tip-of-tree	72.41ms - 74.34ms	unsure 🔍 -2% - +2% -1.67ms - +1.21ms	-	unsure 🔍 -1% - +3% -0.45ms - +2.14ms
previous-release previous-release	71.67ms - 73.39ms	unsure 🔍 -3% - +0% -2.45ms - +0.30ms	unsure 🔍 -3% - +1% -2.14ms - +0.45ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	837.60ms - 857.39ms	-	unsure 🔍 -1% - +2% -7.63ms - +19.71ms	unsure 🔍 -0% - +3% -2.28ms - +27.85ms
tip-of-tree tip-of-tree	832.03ms - 850.89ms	unsure 🔍 -2% - +1% -19.71ms - +7.63ms	-	unsure 🔍 -1% - +3% -8.02ms - +21.52ms
previous-release previous-release	823.35ms - 846.07ms	unsure 🔍 -3% - +0% -27.85ms - +2.28ms	unsure 🔍 -3% - +1% -21.52ms - +8.02ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	902.90ms - 924.60ms	-	unsure 🔍 -1% - +2% -10.24ms - +20.94ms	unsure 🔍 -1% - +2% -11.63ms - +18.29ms
tip-of-tree tip-of-tree	897.21ms - 919.59ms	unsure 🔍 -2% - +1% -20.94ms - +10.24ms	-	unsure 🔍 -2% - +1% -17.24ms - +13.19ms
previous-release previous-release	900.12ms - 920.73ms	unsure 🔍 -2% - +1% -18.29ms - +11.63ms	unsure 🔍 -1% - +2% -13.19ms - +17.24ms	-

_{tachometer-reporter-action v2 for Benchmarks}

sorvell · 2021-08-10T17:33:10Z

packages/lit-html/src/directives/async-append.ts

-      const newPart = insertPart(part);
-      setChildPartValue(newPart, v);
-      i++;
+  // Override point for AsyncReplace to replace rather than append


The comments here are confusing since this is AsyncAppend.

I made AsyncAppend subclass AsyncReplace since it's mostly identical code. As you can see in AsyncAppend, the only differences are:

validatePartType: AsyncReplace can be used anywhere but AsyncAppend can only go in ChildPart position

update: AsyncAppend needs to save the __childPart since it inserts new parts into it

commitValue: AsyncAppend inserts new parts for each value, whereas AsyncReplace just calls setValue and overwrites the previous

kevinpschaaf requested a review from justinfagnani as a code owner August 10, 2021 01:27

google-cla bot added the cla: yes label Aug 10, 2021

kevinpschaaf mentioned this pull request Aug 10, 2021

[lit-html, reactive-element] Revert change to pause update cycle while disconnected #2034

Merged

kevinpschaaf changed the title ~~Re-work async directives for better gc behavior~~ [lit-html] Re-work async directives for better gc behavior Aug 10, 2021

sorvell requested changes Aug 10, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[lit-html] Re-work async directives for better gc behavior #2044

[lit-html] Re-work async directives for better gc behavior #2044

Uh oh!

kevinpschaaf commented Aug 10, 2021

Uh oh!

changeset-bot bot commented Aug 10, 2021 •

edited

Loading

Uh oh!

github-actions bot commented Aug 10, 2021 •

edited

Loading

📊 Tachometer Benchmark Results

Summary

nop-update

render

update

update-reflect

Results

render

update

update-reflect

render

update

nop-update

render

update

render

update

render

update

update-reflect

Uh oh!

sorvell Aug 10, 2021

Uh oh!

kevinpschaaf Aug 11, 2021

Uh oh!

kevinpschaaf Aug 11, 2021

[lit-html] Re-work async directives for better gc behavior #2044

[lit-html] Re-work async directives for better gc behavior #2044

Uh oh!

Conversation

kevinpschaaf commented Aug 10, 2021

Uh oh!

changeset-bot bot commented Aug 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions bot commented Aug 10, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📊 Tachometer Benchmark Results

Summary

nop-update

render

update

update-reflect

Results

render

update

update-reflect

render

update

nop-update

render

update

render

update

render

update

update-reflect

Uh oh!

sorvell Aug 10, 2021

Choose a reason for hiding this comment

Uh oh!

kevinpschaaf Aug 11, 2021

Choose a reason for hiding this comment

Uh oh!

kevinpschaaf Aug 11, 2021

Choose a reason for hiding this comment

changeset-bot bot commented Aug 10, 2021 •

edited

Loading

github-actions bot commented Aug 10, 2021 •

edited

Loading