Remove private Lit 2 migration helpers by AndrewJakubowicz · Pull Request #2828 · lit/lit

AndrewJakubowicz · 2022-05-03T22:26:32Z

Context

While migrating internal to Lit 2, we had these additional options to change how the render function behaved. However these are no longer required as they are not referenced internally.

Testing

Tested internally with a TGP using the same change, and checking for all references of INTERNAL and clearContainerForLit2MigrationOnly. Manually checked that copy.bara wouldn't need changes.

I also removed a unit test that was testing the INTERNAL and clearContainerForLit2MigrationOnly option behavior.

Removed: `INTERNAL` and `clearContainerForLit2MigrationOnly`.

changeset-bot · 2022-05-03T22:26:34Z

🦋 Changeset detected

Latest commit: 94186c6

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 · 2022-05-03T22:28:44Z

📊 Tachometer Benchmark Results

Summary

nop-update

lit-html-kitchen-sink: unsure 🔍 -23% - +24% (-6.67ms - +7.10ms)
^{this-change vs tip-of-tree}

render

lit-element-list: unsure 🔍 -7% - +3% (-5.41ms - +2.19ms)
^{this-change vs tip-of-tree}
lit-html-kitchen-sink: unsure 🔍 -15% - +21% (-4.59ms - +6.45ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -5% - +4% (-0.56ms - +0.45ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -6% - +5% (-3.58ms - +2.77ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -19% - +6% (-16.49ms - +5.54ms)
^{this-change vs tip-of-tree}

update

lit-element-list: unsure 🔍 -5% - +2% (-43.29ms - +19.51ms)
^{this-change vs tip-of-tree}
lit-html-kitchen-sink: unsure 🔍 -14% - +17% (-12.47ms - +14.79ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -9% - +5% (-31.66ms - +16.27ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -10% - +4% (-14.39ms - +5.20ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -9% - +19% (-87.63ms - +194.08ms)
^{this-change vs tip-of-tree}

update-reflect

lit-element-list: unsure 🔍 -2% - +1% (-17.83ms - +5.45ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -4% - +7% (-37.93ms - +55.69ms)
^{this-change vs tip-of-tree}

Results

lit-element-list

Browser: chrome-headless 100.0.4896.127
Sample size: 50
Built by: Benchmarks #2934
Commit: 52cb0b1

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	72.86ms - 75.44ms	-	unsure 🔍 -7% - +3% -5.41ms - +2.19ms	faster ✔ 16% - 20% 14.26ms - 17.96ms
tip-of-tree tip-of-tree	72.18ms - 79.33ms	unsure 🔍 -3% - +7% -2.19ms - +5.41ms	-	faster ✔ 12% - 20% 10.69ms - 18.31ms
previous-release previous-release	88.94ms - 91.58ms	slower ❌ 19% - 24% 14.26ms - 17.96ms	slower ❌ 13% - 25% 10.69ms - 18.31ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	811.96ms - 829.74ms	-	unsure 🔍 -5% - +2% -43.29ms - +19.51ms	faster ✔ 34% - 36% 433.59ms - 468.32ms
tip-of-tree tip-of-tree	802.62ms - 862.85ms	unsure 🔍 -2% - +5% -19.51ms - +43.29ms	-	faster ✔ 32% - 37% 405.46ms - 472.67ms
previous-release previous-release	1256.89ms - 1286.71ms	slower ❌ 52% - 57% 433.59ms - 468.32ms	slower ❌ 47% - 59% 405.46ms - 472.67ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	792.54ms - 808.43ms	-	unsure 🔍 -2% - +1% -17.83ms - +5.45ms	faster ✔ 6% - 9% 50.77ms - 76.40ms
tip-of-tree tip-of-tree	798.17ms - 815.18ms	unsure 🔍 -1% - +2% -5.45ms - +17.83ms	-	faster ✔ 5% - 8% 44.22ms - 70.57ms
previous-release previous-release	854.01ms - 874.13ms	slower ❌ 6% - 10% 50.77ms - 76.40ms	slower ❌ 5% - 9% 44.22ms - 70.57ms	-

lit-html-kitchen-sink

Browser: chrome-headless 100.0.4896.127
Sample size: 30
Built by: Benchmarks #2934
Commit: 52cb0b1

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	28.38ms - 36.68ms	-	unsure 🔍 -15% - +21% -4.59ms - +6.45ms	unsure 🔍 -29% - +0% -11.52ms - +0.56ms
tip-of-tree tip-of-tree	27.96ms - 35.24ms	unsure 🔍 -20% - +14% -6.45ms - +4.59ms	-	faster ✔ 3% - 30% 0.71ms - 12.11ms
previous-release previous-release	33.62ms - 42.40ms	unsure 🔍 -3% - +37% -0.56ms - +11.52ms	slower ❌ 1% - 40% 0.71ms - 12.11ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	79.43ms - 99.45ms	-	unsure 🔍 -14% - +17% -12.47ms - +14.79ms	unsure 🔍 -10% - +17% -8.14ms - +14.91ms
tip-of-tree tip-of-tree	79.02ms - 97.53ms	unsure 🔍 -16% - +14% -14.79ms - +12.47ms	-	unsure 🔍 -10% - +15% -8.65ms - +13.10ms
previous-release previous-release	80.34ms - 91.76ms	unsure 🔍 -16% - +9% -14.91ms - +8.14ms	unsure 🔍 -15% - +10% -13.10ms - +8.65ms	-

nop-update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	25.00ms - 34.86ms	-	unsure 🔍 -23% - +24% -6.67ms - +7.10ms	unsure 🔍 -21% - +17% -6.59ms - +5.26ms
tip-of-tree tip-of-tree	24.91ms - 34.52ms	unsure 🔍 -24% - +22% -7.10ms - +6.67ms	-	unsure 🔍 -22% - +16% -6.71ms - +4.94ms
previous-release previous-release	27.31ms - 33.89ms	unsure 🔍 -18% - +22% -5.26ms - +6.59ms	unsure 🔍 -17% - +23% -4.94ms - +6.71ms	-

lit-html-repeat

Browser: chrome-headless 100.0.4896.127
Sample size: 30
Built by: Benchmarks #2934
Commit: 52cb0b1

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	10.05ms - 10.89ms	-	unsure 🔍 -5% - +4% -0.56ms - +0.45ms	faster ✔ 6% - 14% 0.74ms - 1.70ms
tip-of-tree tip-of-tree	10.25ms - 10.81ms	unsure 🔍 -4% - +5% -0.45ms - +0.56ms	-	faster ✔ 7% - 13% 0.79ms - 1.53ms
previous-release previous-release	11.46ms - 11.92ms	slower ❌ 7% - 17% 0.74ms - 1.70ms	slower ❌ 7% - 15% 0.79ms - 1.53ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	311.19ms - 345.62ms	-	unsure 🔍 -9% - +5% -31.66ms - +16.27ms	faster ✔ 24% - 34% 105.72ms - 161.00ms
tip-of-tree tip-of-tree	319.43ms - 352.77ms	unsure 🔍 -5% - +10% -16.27ms - +31.66ms	-	faster ✔ 22% - 32% 98.36ms - 152.97ms
previous-release previous-release	440.13ms - 483.39ms	slower ❌ 31% - 50% 105.72ms - 161.00ms	slower ❌ 28% - 47% 98.36ms - 152.97ms	-

lit-html-template-heavy

Browser: chrome-headless 100.0.4896.127
Sample size: 50
Built by: Benchmarks #2934
Commit: 52cb0b1

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	54.15ms - 56.44ms	-	unsure 🔍 -6% - +5% -3.58ms - +2.77ms	faster ✔ 12% - 16% 7.76ms - 10.66ms
tip-of-tree tip-of-tree	52.74ms - 58.66ms	unsure 🔍 -5% - +6% -2.77ms - +3.58ms	-	faster ✔ 9% - 18% 5.71ms - 11.89ms
previous-release previous-release	63.61ms - 65.40ms	slower ❌ 14% - 20% 7.76ms - 10.66ms	slower ❌ 9% - 22% 5.71ms - 11.89ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	129.45ms - 133.94ms	-	unsure 🔍 -10% - +4% -14.39ms - +5.20ms	faster ✔ 10% - 14% 14.36ms - 21.00ms
tip-of-tree tip-of-tree	126.75ms - 145.82ms	unsure 🔍 -4% - +11% -5.20ms - +14.39ms	-	faster ✔ 2% - 15% 3.24ms - 22.93ms
previous-release previous-release	146.93ms - 151.82ms	slower ❌ 11% - 16% 14.36ms - 21.00ms	slower ❌ 2% - 17% 3.24ms - 22.93ms	-

reactive-element-list

Browser: chrome-headless 100.0.4896.127
Sample size: 50
Built by: Benchmarks #2934
Commit: 52cb0b1

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	72.02ms - 88.11ms	-	unsure 🔍 -19% - +6% -16.49ms - +5.54ms	unsure 🔍 -17% - +9% -14.54ms - +7.67ms
tip-of-tree tip-of-tree	78.02ms - 93.06ms	unsure 🔍 -7% - +21% -5.54ms - +16.49ms	-	unsure 🔍 -11% - +15% -8.69ms - +12.76ms
previous-release previous-release	75.86ms - 91.15ms	unsure 🔍 -10% - +18% -7.67ms - +14.54ms	unsure 🔍 -15% - +10% -12.76ms - +8.69ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	956.40ms - 1175.76ms	-	unsure 🔍 -9% - +19% -87.63ms - +194.08ms	unsure 🔍 -8% - +19% -79.13ms - +191.79ms
tip-of-tree tip-of-tree	924.47ms - 1101.24ms	unsure 🔍 -18% - +8% -194.08ms - +87.63ms	-	unsure 🔍 -11% - +12% -115.78ms - +121.99ms
previous-release previous-release	930.25ms - 1089.26ms	unsure 🔍 -18% - +7% -191.79ms - +79.13ms	unsure 🔍 -12% - +11% -121.99ms - +115.78ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	821.17ms - 909.05ms	-	unsure 🔍 -4% - +7% -37.93ms - +55.69ms	unsure 🔍 -3% - +7% -28.56ms - +62.61ms
tip-of-tree tip-of-tree	840.10ms - 872.36ms	unsure 🔍 -6% - +4% -55.69ms - +37.93ms	-	unsure 🔍 -1% - +3% -12.04ms - +28.32ms
previous-release previous-release	835.96ms - 860.21ms	unsure 🔍 -7% - +3% -62.61ms - +28.56ms	unsure 🔍 -3% - +1% -28.32ms - +12.04ms	-

_{tachometer-reporter-action v2 for Benchmarks}

rictic

Awesome work!

packages/lit-html/src/test/lit-html_test.ts

AndrewJakubowicz added 2 commits May 3, 2022 15:15

Remove private Lit 2 migration helpers that are no longer depended on.

fe86413

Removed: `INTERNAL` and `clearContainerForLit2MigrationOnly`.

Add changeset

50c08a8

AndrewJakubowicz requested a review from justinfagnani as a code owner May 3, 2022 22:26

AndrewJakubowicz requested a review from aomarks May 3, 2022 22:26

Remove unused import that broke build

c2faa16

AndrewJakubowicz requested a review from rictic May 3, 2022 23:01

rictic approved these changes May 3, 2022

View reviewed changes

justinfagnani requested changes May 4, 2022

View reviewed changes

packages/lit-html/src/test/lit-html_test.ts Outdated Show resolved Hide resolved

Code review feedback - remove internal test

94186c6

AndrewJakubowicz requested a review from justinfagnani May 4, 2022 23:13

justinfagnani approved these changes May 4, 2022

View reviewed changes

AndrewJakubowicz merged commit b3b6bc3 into main May 5, 2022

AndrewJakubowicz deleted the remove-migration-internal-option branch May 5, 2022 01:10

This was referenced May 16, 2022

Version Packages #2912

Closed

Version Packages #2917

Merged

Conversation

AndrewJakubowicz commented May 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Context

Testing

Uh oh!

changeset-bot bot commented May 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

github-actions bot commented May 3, 2022 • 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!

rictic left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

AndrewJakubowicz commented May 3, 2022 •

edited

Loading

changeset-bot bot commented May 3, 2022 •

edited

Loading

github-actions bot commented May 3, 2022 •

edited

Loading