Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

async-replace correctly re-renders when value is unchanged #4409

Merged
merged 3 commits into from Nov 21, 2023

Conversation

sorvell
Copy link
Member

@sorvell sorvell commented Nov 20, 2023

Fixes #4408.

Copy link

changeset-bot bot commented Nov 20, 2023

🦋 Changeset detected

Latest commit: 667a341

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

This PR includes changesets to release 2 packages
Name Type
lit-html Patch
lit 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

Copy link
Contributor

github-actions bot commented Nov 20, 2023

📊 Tachometer Benchmark Results

Summary

nop-update

  • this-change, tip-of-tree, previous-release: unsure 🔍 -5% - +3% (-0.64ms - +0.32ms)
    this-change vs tip-of-tree

render

  • this-change: 47.24ms - 49.38ms
  • this-change, tip-of-tree, previous-release: unsure 🔍 -5% - +2% (-0.95ms - +0.30ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +4% (-0.21ms - +1.27ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +3% (-0.32ms - +1.14ms)
    this-change vs tip-of-tree

update

  • this-change: 501.12ms - 505.33ms
  • this-change, tip-of-tree, previous-release: faster ✔ 2% - 12% (0.61ms - 5.20ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -2% - +2% (-1.39ms - +1.16ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +2% (-2.89ms - +9.15ms)
    this-change vs tip-of-tree

update-reflect

  • this-change: 503.77ms - 508.74ms
  • this-change, tip-of-tree, previous-release: unsure 🔍 -2% - +1% (-8.33ms - +5.56ms)
    this-change vs tip-of-tree

Results

this-change

render

VersionAvg timevs
47.24ms - 49.38ms-

update

VersionAvg timevs
501.12ms - 505.33ms-

update-reflect

VersionAvg timevs
503.77ms - 508.74ms-
this-change, tip-of-tree, previous-release

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
17.87ms - 18.72ms-unsure 🔍
-5% - +2%
-0.95ms - +0.30ms
unsure 🔍
-5% - +2%
-0.97ms - +0.33ms
tip-of-tree
tip-of-tree
18.16ms - 19.08msunsure 🔍
-2% - +5%
-0.30ms - +0.95ms
-unsure 🔍
-4% - +4%
-0.67ms - +0.68ms
previous-release
previous-release
18.12ms - 19.10msunsure 🔍
-2% - +5%
-0.33ms - +0.97ms
unsure 🔍
-4% - +4%
-0.68ms - +0.67ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
38.25ms - 41.27ms-faster ✔
2% - 12%
0.61ms - 5.20ms
unsure 🔍
-9% - +3%
-3.57ms - +1.39ms
tip-of-tree
tip-of-tree
40.93ms - 44.39msslower ❌
1% - 13%
0.61ms - 5.20ms
-unsure 🔍
-2% - +11%
-0.81ms - +4.43ms
previous-release
previous-release
38.88ms - 42.82msunsure 🔍
-4% - +9%
-1.39ms - +3.57ms
unsure 🔍
-10% - +2%
-4.43ms - +0.81ms
-

nop-update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
11.14ms - 11.79ms-unsure 🔍
-5% - +3%
-0.64ms - +0.32ms
unsure 🔍
-6% - +3%
-0.71ms - +0.35ms
tip-of-tree
tip-of-tree
11.28ms - 11.98msunsure 🔍
-3% - +6%
-0.32ms - +0.64ms
-unsure 🔍
-5% - +5%
-0.56ms - +0.53ms
previous-release
previous-release
11.22ms - 12.06msunsure 🔍
-3% - +6%
-0.35ms - +0.71ms
unsure 🔍
-5% - +5%
-0.53ms - +0.56ms
-
this-change, tip-of-tree, previous-release

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
34.18ms - 35.19ms-unsure 🔍
-1% - +4%
-0.21ms - +1.27ms
unsure 🔍
-1% - +3%
-0.29ms - +1.09ms
tip-of-tree
tip-of-tree
33.61ms - 34.70msunsure 🔍
-4% - +1%
-1.27ms - +0.21ms
-unsure 🔍
-2% - +2%
-0.85ms - +0.58ms
previous-release
previous-release
33.82ms - 34.76msunsure 🔍
-3% - +1%
-1.09ms - +0.29ms
unsure 🔍
-2% - +2%
-0.58ms - +0.85ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
68.05ms - 69.37ms-unsure 🔍
-2% - +2%
-1.39ms - +1.16ms
unsure 🔍
-1% - +2%
-0.73ms - +1.25ms
tip-of-tree
tip-of-tree
67.73ms - 69.91msunsure 🔍
-2% - +2%
-1.16ms - +1.39ms
-unsure 🔍
-1% - +2%
-0.94ms - +1.70ms
previous-release
previous-release
67.70ms - 69.18msunsure 🔍
-2% - +1%
-1.25ms - +0.73ms
unsure 🔍
-2% - +1%
-1.70ms - +0.94ms
-
this-change, tip-of-tree, previous-release

render

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
32.87ms - 34.10ms-unsure 🔍
-1% - +3%
-0.32ms - +1.14ms
unsure 🔍
-3% - +3%
-0.96ms - +0.97ms
tip-of-tree
tip-of-tree
32.67ms - 33.48msunsure 🔍
-3% - +1%
-1.14ms - +0.32ms
-unsure 🔍
-4% - +1%
-1.25ms - +0.45ms
previous-release
previous-release
32.73ms - 34.22msunsure 🔍
-3% - +3%
-0.97ms - +0.96ms
unsure 🔍
-1% - +4%
-0.45ms - +1.25ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
515.04ms - 524.86ms-unsure 🔍
-1% - +2%
-2.89ms - +9.15ms
unsure 🔍
-1% - +2%
-3.25ms - +9.45ms
tip-of-tree
tip-of-tree
513.34ms - 520.30msunsure 🔍
-2% - +1%
-9.15ms - +2.89ms
-unsure 🔍
-1% - +1%
-5.35ms - +5.29ms
previous-release
previous-release
512.83ms - 520.88msunsure 🔍
-2% - +1%
-9.45ms - +3.25ms
unsure 🔍
-1% - +1%
-5.29ms - +5.35ms
-

update-reflect

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
518.70ms - 528.53ms-unsure 🔍
-2% - +1%
-8.33ms - +5.56ms
unsure 🔍
-1% - +1%
-4.97ms - +7.31ms
tip-of-tree
tip-of-tree
520.09ms - 529.91msunsure 🔍
-1% - +2%
-5.56ms - +8.33ms
-unsure 🔍
-1% - +2%
-3.58ms - +8.69ms
previous-release
previous-release
518.76ms - 526.13msunsure 🔍
-1% - +1%
-7.31ms - +4.97ms
unsure 🔍
-2% - +1%
-8.69ms - +3.58ms
-

tachometer-reporter-action v2 for Benchmarks

Copy link
Contributor

@AndrewJakubowicz AndrewJakubowicz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great! I suggested some changes to shrink the test to only reproduce the bug.

Thank you!

Copy link
Contributor

The size of lit-html.js and lit-core.min.js are as expected.

Co-authored-by: Andrew Jakubowicz <spyr1014@gmail.com>
Copy link
Member

@augustjk augustjk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@AndrewJakubowicz AndrewJakubowicz merged commit 1af7991 into lit:main Nov 21, 2023
8 checks passed
@lit-robot lit-robot mentioned this pull request Jan 9, 2024
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.

[lit-html] Re-rendering an asyncReplace clears its value.
3 participants