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

Fix the lit-html marker length to be consistently 9 characters. #4570

Merged
merged 4 commits into from Apr 2, 2024

Conversation

rictic
Copy link
Collaborator

@rictic rictic commented Mar 7, 2024

This was always how we intended it to behave, and since it didn't have a reliable length before this change this should be safe.

This was always how we intended it to behave, and since it didn't have a reliable length before this change this should be safe.
Copy link

changeset-bot bot commented Mar 7, 2024

🦋 Changeset detected

Latest commit: 61496f1

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

This PR includes changesets to release 3 packages
Name Type
lit-html Patch
lit Patch
lit-element 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 Mar 7, 2024

📊 Tachometer Benchmark Results

Summary

nop-update

  • this-change, tip-of-tree, previous-release: unsure 🔍 -6% - +5% (-0.67ms - +0.54ms)
    this-change vs tip-of-tree

render

  • this-change: 45.22ms - 47.16ms
  • this-change, tip-of-tree, previous-release: unsure 🔍 -5% - +4% (-0.89ms - +0.66ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -2% - +3% (-0.60ms - +0.92ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -3% - +1% (-0.92ms - +0.16ms)
    this-change vs tip-of-tree

update

  • this-change: 472.08ms - 478.53ms
  • this-change, tip-of-tree, previous-release: unsure 🔍 -9% - +3% (-3.58ms - +1.41ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +2% (-0.82ms - +1.52ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +0% (-3.63ms - +0.71ms)
    this-change vs tip-of-tree

update-reflect

  • this-change: 480.23ms - 485.76ms
  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +0% (-2.57ms - +2.47ms)
    this-change vs tip-of-tree

Results

this-change

render

VersionAvg timevs
45.22ms - 47.16ms-

update

VersionAvg timevs
472.08ms - 478.53ms-

update-reflect

VersionAvg timevs
480.23ms - 485.76ms-
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
18.10ms - 19.30ms-unsure 🔍
-5% - +4%
-0.89ms - +0.66ms
unsure 🔍
-7% - +3%
-1.28ms - +0.50ms
tip-of-tree
tip-of-tree
18.32ms - 19.30msunsure 🔍
-4% - +5%
-0.66ms - +0.89ms
-unsure 🔍
-6% - +3%
-1.10ms - +0.55ms
previous-release
previous-release
18.43ms - 19.74msunsure 🔍
-3% - +7%
-0.50ms - +1.28ms
unsure 🔍
-3% - +6%
-0.55ms - +1.10ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
37.58ms - 40.94ms-unsure 🔍
-9% - +3%
-3.58ms - +1.41ms
unsure 🔍
-7% - +6%
-2.73ms - +2.33ms
tip-of-tree
tip-of-tree
38.50ms - 42.19msunsure 🔍
-4% - +9%
-1.41ms - +3.58ms
-unsure 🔍
-5% - +9%
-1.75ms - +3.53ms
previous-release
previous-release
37.56ms - 41.34msunsure 🔍
-6% - +7%
-2.33ms - +2.73ms
unsure 🔍
-9% - +4%
-3.53ms - +1.75ms
-

nop-update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
10.99ms - 11.61ms-unsure 🔍
-6% - +5%
-0.67ms - +0.54ms
unsure 🔍
-1% - +9%
-0.05ms - +0.94ms
tip-of-tree
tip-of-tree
10.84ms - 11.88msunsure 🔍
-5% - +6%
-0.54ms - +0.67ms
-unsure 🔍
-1% - +11%
-0.14ms - +1.15ms
previous-release
previous-release
10.48ms - 11.24msunsure 🔍
-8% - +0%
-0.94ms - +0.05ms
unsure 🔍
-10% - +1%
-1.15ms - +0.14ms
-
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.93ms - 34.15ms-unsure 🔍
-2% - +3%
-0.60ms - +0.92ms
unsure 🔍
-1% - +4%
-0.36ms - +1.17ms
tip-of-tree
tip-of-tree
32.93ms - 33.83msunsure 🔍
-3% - +2%
-0.92ms - +0.60ms
-unsure 🔍
-1% - +3%
-0.39ms - +0.89ms
previous-release
previous-release
32.68ms - 33.59msunsure 🔍
-3% - +1%
-1.17ms - +0.36ms
unsure 🔍
-3% - +1%
-0.89ms - +0.39ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
67.34ms - 69.42ms-unsure 🔍
-1% - +2%
-0.82ms - +1.52ms
unsure 🔍
-2% - +2%
-1.03ms - +1.63ms
tip-of-tree
tip-of-tree
67.49ms - 68.57msunsure 🔍
-2% - +1%
-1.52ms - +0.82ms
-unsure 🔍
-2% - +1%
-1.04ms - +0.94ms
previous-release
previous-release
67.25ms - 68.90msunsure 🔍
-2% - +1%
-1.63ms - +1.03ms
unsure 🔍
-1% - +2%
-0.94ms - +1.04ms
-
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
30.08ms - 30.80ms-unsure 🔍
-3% - +1%
-0.92ms - +0.16ms
unsure 🔍
-2% - +2%
-0.52ms - +0.46ms
tip-of-tree
tip-of-tree
30.41ms - 31.23msunsure 🔍
-1% - +3%
-0.16ms - +0.92ms
-unsure 🔍
-1% - +3%
-0.18ms - +0.88ms
previous-release
previous-release
30.13ms - 30.81msunsure 🔍
-2% - +2%
-0.46ms - +0.52ms
unsure 🔍
-3% - +1%
-0.88ms - +0.18ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
488.23ms - 491.39ms-unsure 🔍
-1% - +0%
-3.63ms - +0.71ms
unsure 🔍
-0% - +1%
-1.78ms - +2.90ms
tip-of-tree
tip-of-tree
489.78ms - 492.77msunsure 🔍
-0% - +1%
-0.71ms - +3.63ms
-unsure 🔍
-0% - +1%
-0.26ms - +4.30ms
previous-release
previous-release
487.53ms - 490.98msunsure 🔍
-1% - +0%
-2.90ms - +1.78ms
unsure 🔍
-1% - +0%
-4.30ms - +0.26ms
-

update-reflect

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
498.21ms - 501.70ms-unsure 🔍
-1% - +0%
-2.57ms - +2.47ms
unsure 🔍
-0% - +1%
-0.68ms - +4.31ms
tip-of-tree
tip-of-tree
498.18ms - 501.83msunsure 🔍
-0% - +1%
-2.47ms - +2.57ms
-unsure 🔍
-0% - +1%
-0.70ms - +4.42ms
previous-release
previous-release
496.35ms - 499.93msunsure 🔍
-1% - +0%
-4.31ms - +0.68ms
unsure 🔍
-1% - +0%
-4.42ms - +0.70ms
-

tachometer-reporter-action v2 for Benchmarks

Copy link
Contributor

github-actions bot commented Mar 7, 2024

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

packages/lit-html/src/lit-html.ts Outdated Show resolved Hide resolved
Co-authored-by: Justin Fagnani <justinfagnani@google.com>
@rictic
Copy link
Collaborator Author

rictic commented Mar 7, 2024

Done

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.

Nice :D

Copy link
Collaborator Author

@rictic rictic left a comment

Choose a reason for hiding this comment

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

LGTM

@justinfagnani justinfagnani merged commit bd88137 into main Apr 2, 2024
9 checks passed
@justinfagnani justinfagnani deleted the consistent-marker-length branch April 2, 2024 16:17
@lit-robot lit-robot mentioned this pull request Apr 15, 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.

None yet

3 participants