Skip to content

Remove unused internal parameters to requestUpdate#4413

Merged
AndrewJakubowicz merged 3 commits intomainfrom
unused-requestupdate-params
Nov 28, 2023
Merged

Remove unused internal parameters to requestUpdate#4413
AndrewJakubowicz merged 3 commits intomainfrom
unused-requestupdate-params

Conversation

@justinfagnani
Copy link
Copy Markdown
Collaborator

@justinfagnani justinfagnani commented Nov 22, 2023

I noticed this while re-reading some of the code. This was for distinguishing initial values while we were still trying to not reflect initial values for standard decorators.

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Nov 22, 2023 •
edited
Loading

🦋 Changeset detected

Latest commit: 81310e8

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

This PR includes changesets to release 3 packages
Name Type
@lit/reactive-element Patch
lit-element 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

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Nov 22, 2023 •
edited
Loading

📊 Tachometer Benchmark Results

Summary

nop-update

  • this-change, tip-of-tree, previous-release: unsure 🔍 -9% - +0% (-1.03ms - +0.07ms)
    this-change vs tip-of-tree

render

  • this-change: 47.63ms - 50.01ms
  • this-change, tip-of-tree, previous-release: unsure 🔍 -3% - +5% (-0.59ms - +1.01ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +3% (-0.48ms - +1.00ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -2% - +2% (-0.62ms - +0.60ms)
    this-change vs tip-of-tree

update

  • this-change: 499.22ms - 504.28ms
  • this-change, tip-of-tree, previous-release: unsure 🔍 -11% - +2% (-4.40ms - +0.82ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -2% - +2% (-1.41ms - +1.05ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -0% - +1% (-2.41ms - +4.16ms)
    this-change vs tip-of-tree

update-reflect

  • this-change: 499.26ms - 502.88ms
  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +0% (-4.51ms - +1.01ms)
    this-change vs tip-of-tree

Results

this-change

render

VersionAvg timevs
47.63ms - 50.01ms-

update

VersionAvg timevs
499.22ms - 504.28ms-

update-reflect

VersionAvg timevs
499.26ms - 502.88ms-
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.40ms - 19.53ms-unsure 🔍
-3% - +5%
-0.59ms - +1.01ms		unsure 🔍
-6% - +2%
-1.14ms - +0.45ms
tip-of-tree
tip-of-tree		18.19ms - 19.33msunsure 🔍
-5% - +3%
-1.01ms - +0.59ms		-unsure 🔍
-7% - +1%
-1.35ms - +0.25ms
previous-release
previous-release		18.75ms - 19.87msunsure 🔍
-2% - +6%
-0.45ms - +1.14ms		unsure 🔍
-1% - +7%
-0.25ms - +1.35ms		-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree		vs previous-release
previous-release
this-change
37.47ms - 41.11ms-unsure 🔍
-11% - +2%
-4.40ms - +0.82ms		unsure 🔍
-11% - +1%
-4.74ms - +0.54ms
tip-of-tree
tip-of-tree		39.21ms - 42.94msunsure 🔍
-2% - +11%
-0.82ms - +4.40ms		-unsure 🔍
-7% - +6%
-2.98ms - +2.36ms
previous-release
previous-release		39.48ms - 43.29msunsure 🔍
-2% - +12%
-0.54ms - +4.74ms		unsure 🔍
-6% - +7%
-2.36ms - +2.98ms		-

nop-update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree		vs previous-release
previous-release
this-change
10.72ms - 11.54ms-unsure 🔍
-9% - +0%
-1.03ms - +0.07ms		unsure 🔍
-7% - +3%
-0.83ms - +0.31ms
tip-of-tree
tip-of-tree		11.24ms - 11.98msunsure 🔍
-1% - +9%
-0.07ms - +1.03ms		-unsure 🔍
-3% - +7%
-0.33ms - +0.77ms
previous-release
previous-release		10.98ms - 11.80msunsure 🔍
-3% - +8%
-0.31ms - +0.83ms		unsure 🔍
-7% - +3%
-0.77ms - +0.33ms		-
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.10ms - 35.18ms-unsure 🔍
-1% - +3%
-0.48ms - +1.00ms		unsure 🔍
-1% - +3%
-0.28ms - +1.16ms
tip-of-tree
tip-of-tree		33.87ms - 34.88msunsure 🔍
-3% - +1%
-1.00ms - +0.48ms		-unsure 🔍
-1% - +3%
-0.51ms - +0.87ms
previous-release
previous-release		33.73ms - 34.67msunsure 🔍
-3% - +1%
-1.16ms - +0.28ms		unsure 🔍
-3% - +1%
-0.87ms - +0.51ms		-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree		vs previous-release
previous-release
this-change
68.85ms - 70.29ms-unsure 🔍
-2% - +2%
-1.41ms - +1.05ms		unsure 🔍
-1% - +2%
-0.59ms - +1.43ms
tip-of-tree
tip-of-tree		68.75ms - 70.74msunsure 🔍
-2% - +2%
-1.05ms - +1.41ms		-unsure 🔍
-1% - +3%
-0.63ms - +1.83ms
previous-release
previous-release		68.44ms - 69.86msunsure 🔍
-2% - +1%
-1.43ms - +0.59ms		unsure 🔍
-3% - +1%
-1.83ms - +0.63ms		-
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.58ms - 33.43ms-unsure 🔍
-2% - +2%
-0.62ms - +0.60ms		unsure 🔍
-2% - +1%
-0.77ms - +0.39ms
tip-of-tree
tip-of-tree		32.57ms - 33.46msunsure 🔍
-2% - +2%
-0.60ms - +0.62ms		-unsure 🔍
-2% - +1%
-0.78ms - +0.42ms
previous-release
previous-release		32.79ms - 33.60msunsure 🔍
-1% - +2%
-0.39ms - +0.77ms		unsure 🔍
-1% - +2%
-0.42ms - +0.78ms		-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree		vs previous-release
previous-release
this-change
505.73ms - 511.12ms-unsure 🔍
-0% - +1%
-2.41ms - +4.16ms		unsure 🔍
-1% - +1%
-2.79ms - +4.38ms
tip-of-tree
tip-of-tree		505.67ms - 509.42msunsure 🔍
-1% - +0%
-4.16ms - +2.41ms		-unsure 🔍
-1% - +1%
-3.10ms - +2.94ms
previous-release
previous-release		505.27ms - 509.99msunsure 🔍
-1% - +1%
-4.38ms - +2.79ms		unsure 🔍
-1% - +1%
-2.94ms - +3.10ms		-

update-reflect

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree		vs previous-release
previous-release
this-change
510.46ms - 513.54ms-unsure 🔍
-1% - +0%
-4.51ms - +1.01ms		unsure 🔍
-1% - +0%
-3.12ms - +1.42ms
tip-of-tree
tip-of-tree		511.46ms - 516.05msunsure 🔍
-0% - +1%
-1.01ms - +4.51ms		-unsure 🔍
-0% - +1%
-1.94ms - +3.74ms
previous-release
previous-release		511.18ms - 514.52msunsure 🔍
-0% - +1%
-1.42ms - +3.12ms		unsure 🔍
-1% - +0%
-3.74ms - +1.94ms		-

tachometer-reporter-action v2 for Benchmarks

@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Nov 22, 2023 •
edited
Loading

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

AndrewJakubowicz
AndrewJakubowicz approved these changes Nov 22, 2023
View reviewed changes
Copy link
Copy Markdown
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.

LGTM!

Thank you. Just need to update the size and changeset!

augustjk
augustjk approved these changes Nov 22, 2023
View reviewed changes
@AndrewJakubowicz AndrewJakubowicz enabled auto-merge (squash) November 27, 2023 23:47
@AndrewJakubowicz
Copy link
Copy Markdown
Contributor

AndrewJakubowicz commented Nov 27, 2023

I addressed code review feedback on changeset and Lit expected size.
There was a minor GitHub outage that blocked Actions from running so I created an empty commit to kickstart the actions.
Once they pass I'll merge.

@AndrewJakubowicz AndrewJakubowicz merged commit f60a3a2 into main Nov 28, 2023
@AndrewJakubowicz AndrewJakubowicz deleted the unused-requestupdate-params branch November 28, 2023 01:01
@lit-robot lit-robot mentioned this pull request Jan 9, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@AndrewJakubowicz AndrewJakubowicz AndrewJakubowicz approved these changes

@augustjk augustjk augustjk approved these changes

@rictic rictic Awaiting requested review from rictic

@kevinpschaaf kevinpschaaf Awaiting requested review from kevinpschaaf kevinpschaaf is a code owner

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@justinfagnani @AndrewJakubowicz @augustjk