Make ContextRoot deduplicate context requests by element and callback identity by justinfagnani · Pull Request #3451 · lit/lit

justinfagnani · 2022-11-11T16:51:35Z

This deduplicates context requests by element identity and callback identity, so that if an element moves around the DOM, causing it to fire multiple context-request events before a provider is available, its context callback is only called once. This behavior is similar to addEventListener() which will call a callback only once no matter how many times it's added for a single event.

This PR also fixes a potential memory leak by using WeakRefs to hold on to both elements and their context request callbacks. We need WeakRefs because when we receive a context-provider event we need to loop over all element/callback pairs for that context and dispatch events. WeakMaps do not allow iterating over entries.

cc @benjamind

changeset-bot · 2022-11-11T16:51:37Z

🦋 Changeset detected

Latest commit: e7abc22

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

This PR includes changesets to release 1 package

Name	Type
@lit-labs/context	Patch

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-11-11T16:51:57Z

📊 Tachometer Benchmark Results

Summary

nop-update

lit-html-kitchen-sink: unsure 🔍 -5% - +12% (-1.23ms - +3.03ms)
^{this-change vs tip-of-tree}

render

lit-element-list: 75.93ms - 80.58ms
lit-html-kitchen-sink: unsure 🔍 -7% - +4% (-2.20ms - +1.34ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -4% - +5% (-0.47ms - +0.66ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -2% - +2% (-1.35ms - +0.90ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -4% - +1% (-1.93ms - +0.70ms)
^{this-change vs tip-of-tree}

update

lit-element-list: 781.99ms - 797.32ms
lit-html-kitchen-sink: unsure 🔍 -4% - +5% (-2.91ms - +4.13ms)
^{this-change vs tip-of-tree}
lit-html-repeat: unsure 🔍 -2% - +2% (-7.30ms - +6.34ms)
^{this-change vs tip-of-tree}
lit-html-template-heavy: unsure 🔍 -1% - +3% (-1.26ms - +3.89ms)
^{this-change vs tip-of-tree}
reactive-element-list: unsure 🔍 -1% - +2% (-11.56ms - +14.23ms)
^{this-change vs tip-of-tree}

update-reflect

lit-element-list: 775.23ms - 785.08ms
reactive-element-list: slower ❌ 1% - 3% (5.10ms - 25.47ms)
^{this-change vs tip-of-tree}

Results

lit-element-list

Browser: chrome-headless 109.0.5414.119
Sample size: 50
Built by: Benchmarks #4356
Commit: e9fe016

render

Version	Avg time	vs
	75.93ms - 80.58ms	-

update

Version	Avg time	vs
	781.99ms - 797.32ms	-

update-reflect

Version	Avg time	vs
	775.23ms - 785.08ms	-

lit-html-kitchen-sink

Browser: chrome-headless 109.0.5414.119
Sample size: 30
Built by: Benchmarks #4356
Commit: e9fe016

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	31.31ms - 33.28ms	-	unsure 🔍 -7% - +4% -2.20ms - +1.34ms	unsure 🔍 -2% - +6% -0.52ms - +2.03ms
tip-of-tree tip-of-tree	31.25ms - 34.19ms	unsure 🔍 -4% - +7% -1.34ms - +2.20ms	-	unsure 🔍 -2% - +9% -0.49ms - +2.87ms
previous-release previous-release	30.73ms - 32.34ms	unsure 🔍 -6% - +2% -2.03ms - +0.52ms	unsure 🔍 -9% - +1% -2.87ms - +0.49ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	78.26ms - 83.27ms	-	unsure 🔍 -4% - +5% -2.91ms - +4.13ms	unsure 🔍 -5% - +4% -4.23ms - +3.02ms
tip-of-tree tip-of-tree	77.69ms - 82.62ms	unsure 🔍 -5% - +4% -4.13ms - +2.91ms	-	unsure 🔍 -6% - +3% -4.81ms - +2.38ms
previous-release previous-release	78.75ms - 83.98ms	unsure 🔍 -4% - +5% -3.02ms - +4.23ms	unsure 🔍 -3% - +6% -2.38ms - +4.81ms	-

nop-update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	24.37ms - 27.01ms	-	unsure 🔍 -5% - +12% -1.23ms - +3.03ms	unsure 🔍 -7% - +9% -1.70ms - +2.22ms
tip-of-tree tip-of-tree	23.12ms - 26.46ms	unsure 🔍 -12% - +5% -3.03ms - +1.23ms	-	unsure 🔍 -11% - +6% -2.85ms - +1.57ms
previous-release previous-release	23.98ms - 26.88ms	unsure 🔍 -9% - +7% -2.22ms - +1.70ms	unsure 🔍 -6% - +12% -1.57ms - +2.85ms	-

lit-html-repeat

Browser: chrome-headless 109.0.5414.119
Sample size: 30
Built by: Benchmarks #4356
Commit: e9fe016

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	11.62ms - 12.49ms	-	unsure 🔍 -4% - +5% -0.47ms - +0.66ms	unsure 🔍 -4% - +7% -0.46ms - +0.79ms
tip-of-tree tip-of-tree	11.61ms - 12.31ms	unsure 🔍 -5% - +4% -0.66ms - +0.47ms	-	unsure 🔍 -4% - +5% -0.50ms - +0.64ms
previous-release previous-release	11.44ms - 12.34ms	unsure 🔍 -7% - +4% -0.79ms - +0.46ms	unsure 🔍 -5% - +4% -0.64ms - +0.50ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	305.07ms - 314.39ms	-	unsure 🔍 -2% - +2% -7.30ms - +6.34ms	unsure 🔍 -2% - +2% -7.15ms - +6.37ms
tip-of-tree tip-of-tree	305.24ms - 315.19ms	unsure 🔍 -2% - +2% -6.34ms - +7.30ms	-	unsure 🔍 -2% - +2% -6.89ms - +7.07ms
previous-release previous-release	305.22ms - 315.02ms	unsure 🔍 -2% - +2% -6.37ms - +7.15ms	unsure 🔍 -2% - +2% -7.07ms - +6.89ms	-

lit-html-template-heavy

Browser: chrome-headless 109.0.5414.119
Sample size: 50
Built by: Benchmarks #4356
Commit: e9fe016

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	53.07ms - 54.33ms	-	unsure 🔍 -2% - +2% -1.35ms - +0.90ms	unsure 🔍 -2% - +2% -0.84ms - +1.08ms
tip-of-tree tip-of-tree	52.99ms - 54.85ms	unsure 🔍 -2% - +3% -0.90ms - +1.35ms	-	unsure 🔍 -2% - +3% -0.83ms - +1.52ms
previous-release previous-release	52.86ms - 54.30ms	unsure 🔍 -2% - +2% -1.08ms - +0.84ms	unsure 🔍 -3% - +2% -1.52ms - +0.83ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	115.02ms - 118.57ms	-	unsure 🔍 -1% - +3% -1.26ms - +3.89ms	unsure 🔍 -1% - +3% -0.88ms - +3.64ms
tip-of-tree tip-of-tree	113.62ms - 117.34ms	unsure 🔍 -3% - +1% -3.89ms - +1.26ms	-	unsure 🔍 -2% - +2% -2.27ms - +2.39ms
previous-release previous-release	114.02ms - 116.82ms	unsure 🔍 -3% - +1% -3.64ms - +0.88ms	unsure 🔍 -2% - +2% -2.39ms - +2.27ms	-

reactive-element-list

Browser: chrome-headless 109.0.5414.119
Sample size: 50
Built by: Benchmarks #4356
Commit: e9fe016

render

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	51.42ms - 53.42ms	-	unsure 🔍 -4% - +1% -1.93ms - +0.70ms	unsure 🔍 -3% - +2% -1.53ms - +0.97ms
tip-of-tree tip-of-tree	52.18ms - 53.89ms	unsure 🔍 -1% - +4% -0.70ms - +1.93ms	-	unsure 🔍 -2% - +3% -0.81ms - +1.48ms
previous-release previous-release	51.94ms - 53.46ms	unsure 🔍 -2% - +3% -0.97ms - +1.53ms	unsure 🔍 -3% - +2% -1.48ms - +0.81ms	-

update

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	831.37ms - 849.77ms	-	unsure 🔍 -1% - +2% -11.56ms - +14.23ms	unsure 🔍 -2% - +2% -13.71ms - +13.37ms
tip-of-tree tip-of-tree	830.20ms - 848.27ms	unsure 🔍 -2% - +1% -14.23ms - +11.56ms	-	unsure 🔍 -2% - +1% -14.93ms - +11.92ms
previous-release previous-release	830.81ms - 850.67ms	unsure 🔍 -2% - +2% -13.37ms - +13.71ms	unsure 🔍 -1% - +2% -11.92ms - +14.93ms	-

update-reflect

Version	Avg time	vs this-change	vs tip-of-tree tip-of-tree	vs previous-release previous-release
this-change	860.55ms - 870.27ms	-	slower ❌ 1% - 3% 5.10ms - 25.47ms	slower ❌ 0% - 3% 1.90ms - 21.34ms
tip-of-tree tip-of-tree	841.18ms - 859.07ms	faster ✔ 1% - 3% 5.10ms - 25.47ms	-	unsure 🔍 -2% - +1% -15.95ms - +8.62ms
previous-release previous-release	845.37ms - 862.21ms	faster ✔ 0% - 2% 1.90ms - 21.34ms	unsure 🔍 -1% - +2% -8.62ms - +15.95ms	-

_{tachometer-reporter-action v2 for Benchmarks}

packages/labs/context/src/lib/controllers/context-consumer.ts

packages/labs/context/src/test/late-provider_test.ts

kevinpschaaf

Let's either skip Context tests on IE11 and document that the Context package doesn't work on IE11 / older browsers, or else document needing a WeakRef polyfill

This gets us Safari 14

justinfagnani · 2023-01-31T20:40:06Z

@kevinpschaaf PTAL

kevinpschaaf

LGTM

packages/labs/context/src/lib/context-root.ts

Co-authored-by: Kevin Schaaf <kschaaf@google.com>

justinfagnani added 5 commits November 11, 2022 08:29

Rename ev to event

a6c5d5e

WIP

63cc153

ContextRoot: dedupe callbacks

b4eddb5

Add changeset

5efbfa8

Update implementation to deduplicate on request

5b32dce

justinfagnani requested review from kevinpschaaf and sorvell November 11, 2022 16:51

justinfagnani commented Nov 11, 2022

View reviewed changes

packages/labs/context/src/lib/controllers/context-consumer.ts Show resolved Hide resolved

packages/labs/context/src/test/late-provider_test.ts Show resolved Hide resolved

kevinpschaaf reviewed Nov 16, 2022

View reviewed changes

justinfagnani added 2 commits January 27, 2023 16:45

Update Context readme

fee86e4

Skip ContextRoot tests on IE

5387891

justinfagnani requested a review from kevinpschaaf January 28, 2023 01:29

Update macos test version to macos 11

3322a00

This gets us Safari 14

kevinpschaaf approved these changes Feb 2, 2023

View reviewed changes

packages/labs/context/src/lib/context-root.ts Outdated Show resolved Hide resolved

Update packages/labs/context/src/lib/context-root.ts

e7abc22

Co-authored-by: Kevin Schaaf <kschaaf@google.com>

justinfagnani enabled auto-merge (squash) February 2, 2023 00:55

auto-merge was automatically disabled February 4, 2023 01:51
Base branch requires signed commits

justinfagnani merged commit 7c93499 into main Feb 4, 2023

justinfagnani deleted the context-root-bug branch February 4, 2023 02:00

lit-robot mentioned this pull request Mar 10, 2023

Version Packages #3717

Merged

Conversation

justinfagnani commented Nov 11, 2022

Uh oh!

changeset-bot bot commented Nov 11, 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 Nov 11, 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!

Uh oh!

Uh oh!

kevinpschaaf left a comment

Choose a reason for hiding this comment

Uh oh!

justinfagnani commented Jan 31, 2023

Uh oh!

kevinpschaaf left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

changeset-bot bot commented Nov 11, 2022 •

edited

Loading

github-actions bot commented Nov 11, 2022 •

edited

Loading