Skip to content

[investigation] memory leak repro in SSR#4514

Draft
AndrewJakubowicz wants to merge 2 commits intomainfrom
ssr-memory-leak
Draft

[investigation] memory leak repro in SSR#4514
AndrewJakubowicz wants to merge 2 commits intomainfrom
ssr-memory-leak

Conversation

@AndrewJakubowicz
Copy link
Copy Markdown
Contributor

Context

This PR reproduces a memory leak in SSR that only occurs with directives.

The memory leak is in patchedDirectiveCache.

@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Jan 23, 2024

⚠️ No Changeset found

Latest commit: 1c84583

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@AndrewJakubowicz AndrewJakubowicz marked this pull request as draft January 23, 2024 23:11
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Jan 23, 2024

📊 Tachometer Benchmark Results

Summary

nop-update

  • this-change, tip-of-tree, previous-release: faster ✔ 1% - 11% (0.10ms - 1.32ms)
    this-change vs tip-of-tree

render

  • this-change: 51.17ms - 53.95ms
  • this-change, tip-of-tree, previous-release: unsure 🔍 -6% - +5% (-1.08ms - +1.01ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -2% - +2% (-0.75ms - +0.65ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -3% - +1% (-0.97ms - +0.31ms)
    this-change vs tip-of-tree

update

  • this-change: 561.04ms - 573.93ms
  • this-change, tip-of-tree, previous-release: unsure 🔍 -10% - +3% (-4.70ms - +1.28ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -2% - +2% (-1.50ms - +1.31ms)
    this-change vs tip-of-tree
  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +1% (-3.11ms - +4.87ms)
    this-change vs tip-of-tree

update-reflect

  • this-change: 560.92ms - 569.66ms
  • this-change, tip-of-tree, previous-release: unsure 🔍 -1% - +1% (-6.44ms - +3.76ms)
    this-change vs tip-of-tree

Results

this-change

render

VersionAvg timevs
51.17ms - 53.95ms-

update

VersionAvg timevs
561.04ms - 573.93ms-

update-reflect

VersionAvg timevs
560.92ms - 569.66ms-
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.66ms - 20.34ms-unsure 🔍
-6% - +5%
-1.08ms - +1.01ms
unsure 🔍
-4% - +6%
-0.75ms - +1.13ms
tip-of-tree
tip-of-tree
18.91ms - 20.16msunsure 🔍
-5% - +6%
-1.01ms - +1.08ms
-unsure 🔍
-3% - +5%
-0.54ms - +0.98ms
previous-release
previous-release
18.89ms - 19.74msunsure 🔍
-6% - +4%
-1.13ms - +0.75ms
unsure 🔍
-5% - +3%
-0.98ms - +0.54ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
41.98ms - 46.46ms-unsure 🔍
-10% - +3%
-4.70ms - +1.28ms
unsure 🔍
-5% - +8%
-2.39ms - +3.28ms
tip-of-tree
tip-of-tree
43.95ms - 47.92msunsure 🔍
-3% - +11%
-1.28ms - +4.70ms
-unsure 🔍
-1% - +11%
-0.48ms - +4.80ms
previous-release
previous-release
42.04ms - 45.51msunsure 🔍
-7% - +5%
-3.28ms - +2.39ms
unsure 🔍
-10% - +1%
-4.80ms - +0.48ms
-

nop-update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
11.23ms - 12.07ms-faster ✔
1% - 11%
0.10ms - 1.32ms
unsure 🔍
-6% - +3%
-0.73ms - +0.41ms
tip-of-tree
tip-of-tree
11.92ms - 12.80msslower ❌
1% - 12%
0.10ms - 1.32ms
-unsure 🔍
-0% - +10%
-0.03ms - +1.14ms
previous-release
previous-release
11.42ms - 12.19msunsure 🔍
-4% - +6%
-0.41ms - +0.73ms
unsure 🔍
-9% - +0%
-1.14ms - +0.03ms
-
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
35.08ms - 36.07ms-unsure 🔍
-2% - +2%
-0.75ms - +0.65ms
unsure 🔍
-1% - +3%
-0.29ms - +1.10ms
tip-of-tree
tip-of-tree
35.13ms - 36.12msunsure 🔍
-2% - +2%
-0.65ms - +0.75ms
-unsure 🔍
-1% - +3%
-0.24ms - +1.15ms
previous-release
previous-release
34.68ms - 35.66msunsure 🔍
-3% - +1%
-1.10ms - +0.29ms
unsure 🔍
-3% - +1%
-1.15ms - +0.24ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
83.19ms - 85.11ms-unsure 🔍
-2% - +2%
-1.50ms - +1.31ms
slower ❌
0% - 4%
0.18ms - 3.05ms
tip-of-tree
tip-of-tree
83.22ms - 85.26msunsure 🔍
-2% - +2%
-1.31ms - +1.50ms
-slower ❌
0% - 4%
0.23ms - 3.19ms
previous-release
previous-release
81.47ms - 83.60msfaster ✔
0% - 4%
0.18ms - 3.05ms
faster ✔
0% - 4%
0.23ms - 3.19ms
-
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.80ms - 35.74ms-unsure 🔍
-3% - +1%
-0.97ms - +0.31ms
unsure 🔍
-2% - +2%
-0.66ms - +0.77ms
tip-of-tree
tip-of-tree
35.16ms - 36.04msunsure 🔍
-1% - +3%
-0.31ms - +0.97ms
-unsure 🔍
-1% - +3%
-0.31ms - +1.09ms
previous-release
previous-release
34.67ms - 35.76msunsure 🔍
-2% - +2%
-0.77ms - +0.66ms
unsure 🔍
-3% - +1%
-1.09ms - +0.31ms
-

update

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
597.14ms - 603.09ms-unsure 🔍
-1% - +1%
-3.11ms - +4.87ms
unsure 🔍
-1% - +1%
-4.86ms - +3.79ms
tip-of-tree
tip-of-tree
596.57ms - 601.89msunsure 🔍
-1% - +1%
-4.87ms - +3.11ms
-unsure 🔍
-1% - +0%
-5.54ms - +2.70ms
previous-release
previous-release
597.51ms - 603.79msunsure 🔍
-1% - +1%
-3.79ms - +4.86ms
unsure 🔍
-0% - +1%
-2.70ms - +5.54ms
-

update-reflect

VersionAvg timevs this-change
vs tip-of-tree
tip-of-tree
vs previous-release
previous-release
this-change
597.73ms - 606.45ms-unsure 🔍
-1% - +1%
-6.44ms - +3.76ms
unsure 🔍
-1% - +1%
-5.49ms - +5.89ms
tip-of-tree
tip-of-tree
600.80ms - 606.07msunsure 🔍
-1% - +1%
-3.76ms - +6.44ms
-unsure 🔍
-0% - +1%
-2.97ms - +6.05ms
previous-release
previous-release
598.24ms - 605.55msunsure 🔍
-1% - +1%
-5.89ms - +5.49ms
unsure 🔍
-1% - +0%
-6.05ms - +2.97ms
-

tachometer-reporter-action v2 for Benchmarks

@github-actions
Copy link
Copy Markdown
Contributor

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

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.

1 participant