Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

skip comment nodes for placeChild #4128

Merged
merged 3 commits into from
Dec 29, 2023
Merged

skip comment nodes for placeChild #4128

merged 3 commits into from
Dec 29, 2023

Conversation

JoviDeCroock
Copy link
Member

@JoviDeCroock JoviDeCroock commented Sep 7, 2023 •
edited
Loading

Supersedes #3771
Enables preactjs/preact-render-to-string#296

currently the comment-nodes that we use so our custom-element can attach the new dom in the right place gets removed during hydration and gets a few insertBefore, ... You can try this branch out in a streaming renderer scenario by

@github-actions
Copy link

github-actions bot commented Sep 7, 2023 •
edited
Loading

📊 Tachometer Benchmark Results

Summary

duration

  • 02_replace1k: unsure 🔍 -1% - +2% (-0.67ms - +1.76ms)
    preact-local vs preact-main
  • 03_update10th1k_x16: unsure 🔍 -3% - +5% (-1.03ms - +1.50ms)
    preact-local vs preact-main
  • 07_create10k: unsure 🔍 -0% - +0% (-4.47ms - +3.47ms)
    preact-local vs preact-main
  • filter_list: unsure 🔍 -0% - +1% (-0.02ms - +0.13ms)
    preact-local vs preact-main
  • hydrate1k: unsure 🔍 -0% - +2% (-0.19ms - +1.52ms)
    preact-local vs preact-main
  • many_updates: unsure 🔍 -3% - +4% (-0.54ms - +0.74ms)
    preact-local vs preact-main
  • text_update: unsure 🔍 -1% - +4% (-0.02ms - +0.10ms)
    preact-local vs preact-main
  • todo: slower ❌ 0% - 2% (0.04ms - 0.55ms)
    preact-local vs preact-main

usedJSHeapSize

  • 02_replace1k: unsure 🔍 -0% - +0% (-0.00ms - +0.01ms)
    preact-local vs preact-main
  • 03_update10th1k_x16: unsure 🔍 -0% - +0% (-0.00ms - +0.01ms)
    preact-local vs preact-main
  • 07_create10k: unsure 🔍 +0% - +0% (+0.00ms - +0.00ms)
    preact-local vs preact-main
  • filter_list: unsure 🔍 +0% - +0% (+0.00ms - +0.01ms)
    preact-local vs preact-main
  • hydrate1k: unsure 🔍 +0% - +0% (+0.00ms - +0.01ms)
    preact-local vs preact-main
  • many_updates: unsure 🔍 +0% - +0% (+0.00ms - +0.01ms)
    preact-local vs preact-main
  • text_update: unsure 🔍 -2% - +0% (-0.01ms - +0.00ms)
    preact-local vs preact-main
  • todo: unsure 🔍 +0% - +0% (+0.00ms - +0.00ms)
    preact-local vs preact-main

Results

02_replace1k

duration

VersionAvg timevs preact-mainvs preact-localvs preact-hooks
preact-main79.29ms - 80.68ms-unsure 🔍
-2% - +1%
-1.76ms - +0.67ms		unsure 🔍
-1% - +1%
-1.09ms - +0.98ms
preact-local79.54ms - 81.52msunsure 🔍
-1% - +2%
-0.67ms - +1.76ms		-unsure 🔍
-1% - +2%
-0.76ms - +1.74ms
preact-hooks79.27ms - 80.81msunsure 🔍
-1% - +1%
-0.98ms - +1.09ms		unsure 🔍
-2% - +1%
-1.74ms - +0.76ms		-

usedJSHeapSize

VersionAvg timevs preact-mainvs preact-localvs preact-hooks
preact-main3.31ms - 3.32ms-unsure 🔍
-0% - +0%
-0.01ms - +0.00ms		faster ✔
1% - 1%
0.02ms - 0.03ms
preact-local3.32ms - 3.32msunsure 🔍
-0% - +0%
-0.00ms - +0.01ms		-faster ✔
1% - 1%
0.02ms - 0.03ms
preact-hooks3.34ms - 3.34msslower ❌
1% - 1%
0.02ms - 0.03ms		slower ❌
1% - 1%
0.02ms - 0.03ms		-

run-warmup-0

VersionAvg timevs preact-mainvs preact-localvs preact-hooks
preact-main31.35ms - 32.77ms-unsure 🔍
-3% - +3%
-0.96ms - +0.94ms		unsure 🔍
-3% - +2%
-1.05ms - +0.79ms
preact-local31.44ms - 32.70msunsure 🔍
-3% - +3%
-0.94ms - +0.96ms		-unsure 🔍
-3% - +2%
-0.98ms - +0.74ms
preact-hooks31.60ms - 32.77msunsure 🔍
-2% - +3%
-0.79ms - +1.05ms		unsure 🔍
-2% - +3%
-0.74ms - +0.98ms		-

run-warmup-1

VersionAvg timevs preact-mainvs preact-localvs preact-hooks
preact-main38.33ms - 39.86ms-unsure 🔍
-3% - +3%
-1.06ms - +1.03ms		unsure 🔍
-3% - +1%
-1.33ms - +0.46ms
preact-local38.40ms - 39.82msunsure 🔍
-3% - +3%
-1.03ms - +1.06ms		-unsure 🔍
-3% - +1%
-1.27ms - +0.43ms
preact-hooks39.07ms - 40.00msunsure 🔍
-1% - +3%
-0.46ms - +1.33ms		unsure 🔍
-1% - +3%
-0.43ms - +1.27ms		-

run-warmup-2

VersionAvg timevs preact-mainvs preact-localvs preact-hooks
preact-main28.42ms - 29.22ms-unsure 🔍
-2% - +2%
-0.59ms - +0.50ms		unsure 🔍
-4% - +1%
-1.11ms - +0.19ms
preact-local28.49ms - 29.24msunsure 🔍
-2% - +2%
-0.50ms - +0.59ms		-unsure 🔍
-4% - +1%
-1.05ms - +0.22ms
preact-hooks28.77ms - 29.79msunsure 🔍
-1% - +4%
-0.19ms - +1.11ms		unsure 🔍
-1% - +4%
-0.22ms - +1.05ms		-

run-warmup-3

VersionAvg timevs preact-mainvs preact-localvs preact-hooks
preact-main22.34ms - 22.73ms-unsure 🔍
-1% - +1%
-0.24ms - +0.23ms		unsure 🔍
-4% - +0%
-0.82ms - +0.02ms
preact-local22.40ms - 22.68msunsure 🔍
-1% - +1%
-0.23ms - +0.24ms		-faster ✔
0% - 3%
0.00ms - 0.79ms
preact-hooks22.56ms - 23.31msunsure 🔍
-0% - +4%
-0.02ms - +0.82ms		unsure 🔍
-0% - +4%
+0.00ms - +0.79ms		-

run-warmup-4

VersionAvg timevs preact-mainvs preact-localvs preact-hooks
preact-main36.65ms - 38.08ms-unsure 🔍
-3% - +2%
-1.29ms - +0.68ms		faster ✔
1% - 5%
0.28ms - 2.05ms
preact-local37.00ms - 38.34msunsure 🔍
-2% - +3%
-0.68ms - +1.29ms		-faster ✔
0% - 4%
0.01ms - 1.70ms
preact-hooks38.01ms - 39.04msslower ❌
1% - 6%
0.28ms - 2.05ms		slower ❌
0% - 5%
0.01ms - 1.70ms		-

run-final

VersionAvg timevs preact-mainvs preact-localvs preact-hooks
preact-main22.20ms - 22.78ms-unsure 🔍
-3% - +1%
-0.65ms - +0.31ms		unsure 🔍
-3% - +1%
-0.61ms - +0.22ms
preact-local22.28ms - 23.04msunsure 🔍
-1% - +3%
-0.31ms - +0.65ms		-unsure 🔍
-2% - +2%
-0.51ms - +0.46ms
preact-hooks22.38ms - 22.98msunsure 🔍
-1% - +3%
-0.22ms - +0.61ms		unsure 🔍
-2% - +2%
-0.46ms - +0.51ms		-
03_update10th1k_x16

duration

VersionAvg timevs preact-mainvs preact-localvs preact-hooks
preact-main31.57ms - 33.34ms-unsure 🔍
-5% - +3%
-1.50ms - +1.03ms		unsure 🔍
-3% - +5%
-1.06ms - +1.46ms
preact-local31.79ms - 33.59msunsure 🔍
-3% - +5%
-1.03ms - +1.50ms		-unsure 🔍
-3% - +5%
-0.83ms - +1.71ms
preact-hooks31.36ms - 33.15msunsure 🔍
-4% - +3%
-1.46ms - +1.06ms		unsure 🔍
-5% - +3%
-1.71ms - +0.83ms		-

usedJSHeapSize

VersionAvg timevs preact-mainvs preact-localvs preact-hooks
preact-main3.28ms - 3.29ms-unsure 🔍
-0% - +0%
-0.01ms - +0.00ms		faster ✔
0% - 1%
0.01ms - 0.03ms
preact-local3.28ms - 3.29msunsure 🔍
-0% - +0%
-0.00ms - +0.01ms		-faster ✔
0% - 1%
0.01ms - 0.02ms
preact-hooks3.30ms - 3.31msslower ❌
0% - 1%
0.01ms - 0.03ms		slower ❌
0% - 1%
0.01ms - 0.02ms		-
07_create10k

duration

VersionAvg timevs preact-mainvs preact-localvs preact-hooks
preact-main927.01ms - 932.88ms-unsure 🔍
-0% - +0%
-3.47ms - +4.47ms		unsure 🔍
-0% - +0%
-3.57ms - +3.11ms
preact-local926.77ms - 932.12msunsure 🔍
-0% - +0%
-4.47ms - +3.47ms		-unsure 🔍
-0% - +0%
-3.85ms - +2.37ms
preact-hooks928.59ms - 931.77msunsure 🔍
-0% - +0%
-3.11ms - +3.57ms		unsure 🔍
-0% - +0%
-2.37ms - +3.85ms		-

usedJSHeapSize

VersionAvg timevs preact-mainvs preact-localvs preact-hooks
preact-main26.32ms - 26.32ms-unsure 🔍
-0% - -0%
-0.00ms - -0.00ms		unsure 🔍
-0% - -0%
-0.02ms - -0.02ms
preact-local26.32ms - 26.32msunsure 🔍
+0% - +0%
+0.00ms - +0.00ms		-unsure 🔍
-0% - -0%
-0.02ms - -0.02ms
preact-hooks26.34ms - 26.34msunsure 🔍
+0% - +0%
+0.02ms - +0.02ms		unsure 🔍
+0% - +0%
+0.02ms - +0.02ms		-
filter_list

duration

VersionAvg timevs preact-mainvs preact-localvs preact-hooks
preact-main16.60ms - 16.69ms-unsure 🔍
-1% - +0%
-0.13ms - +0.02ms		unsure 🔍
-1% - +0%
-0.16ms - +0.02ms
preact-local16.64ms - 16.77msunsure 🔍
-0% - +1%
-0.02ms - +0.13ms		-unsure 🔍
-1% - +1%
-0.11ms - +0.08ms
preact-hooks16.64ms - 16.79msunsure 🔍
-0% - +1%
-0.02ms - +0.16ms		unsure 🔍
-1% - +1%
-0.08ms - +0.11ms		-

usedJSHeapSize

VersionAvg timevs preact-mainvs preact-localvs preact-hooks
preact-main1.33ms - 1.33ms-unsure 🔍
-0% - -0%
-0.01ms - -0.00ms		faster ✔
2% - 2%
0.03ms - 0.03ms
preact-local1.33ms - 1.33msunsure 🔍
+0% - +0%
+0.00ms - +0.01ms		-faster ✔
2% - 2%
0.02ms - 0.03ms
preact-hooks1.36ms - 1.36msslower ❌
2% - 2%
0.03ms - 0.03ms		slower ❌
2% - 2%
0.02ms - 0.03ms		-
hydrate1k

duration

VersionAvg timevs preact-mainvs preact-localvs preact-hooks
preact-main84.76ms - 85.60ms-unsure 🔍
-2% - +0%
-1.52ms - +0.19ms		unsure 🔍
-5% - +0%
-4.11ms - +0.43ms
preact-local85.10ms - 86.59msunsure 🔍
-0% - +2%
-0.19ms - +1.52ms		-unsure 🔍
-4% - +1%
-3.53ms - +1.17ms
preact-hooks84.79ms - 89.25msunsure 🔍
-1% - +5%
-0.43ms - +4.11ms		unsure 🔍
-1% - +4%
-1.17ms - +3.53ms		-

usedJSHeapSize

VersionAvg timevs preact-mainvs preact-localvs preact-hooks
preact-main6.09ms - 6.10ms-unsure 🔍
-0% - -0%
-0.01ms - -0.00ms		faster ✔
0% - 1%
0.03ms - 0.03ms
preact-local6.10ms - 6.10msunsure 🔍
+0% - +0%
+0.00ms - +0.01ms		-unsure 🔍
-0% - -0%
-0.03ms - -0.02ms
preact-hooks6.12ms - 6.13msslower ❌
0% - 1%
0.03ms - 0.03ms		unsure 🔍
+0% - +0%
+0.02ms - +0.03ms		-
many_updates

duration

VersionAvg timevs preact-mainvs preact-localvs preact-hooks
preact-main16.49ms - 17.31ms-unsure 🔍
-4% - +3%
-0.74ms - +0.54ms		unsure 🔍
-2% - +4%
-0.37ms - +0.63ms
preact-local16.51ms - 17.49msunsure 🔍
-3% - +4%
-0.54ms - +0.74ms		-unsure 🔍
-2% - +5%
-0.34ms - +0.80ms
preact-hooks16.49ms - 17.06msunsure 🔍
-4% - +2%
-0.63ms - +0.37ms		unsure 🔍
-5% - +2%
-0.80ms - +0.34ms		-

usedJSHeapSize

VersionAvg timevs preact-mainvs preact-localvs preact-hooks
preact-main4.43ms - 4.44ms-unsure 🔍
-0% - -0%
-0.01ms - -0.00ms		faster ✔
0% - 1%
0.02ms - 0.03ms
preact-local4.44ms - 4.44msunsure 🔍
+0% - +0%
+0.00ms - +0.01ms		-unsure 🔍
-0% - -0%
-0.02ms - -0.01ms
preact-hooks4.46ms - 4.46msslower ❌
0% - 1%
0.02ms - 0.03ms		unsure 🔍
+0% - +0%
+0.01ms - +0.02ms		-
text_update

duration

VersionAvg timevs preact-mainvs preact-localvs preact-hooks
preact-main2.50ms - 2.58ms-unsure 🔍
-4% - +1%
-0.10ms - +0.02ms		faster ✔
5% - 9%
0.15ms - 0.26ms
preact-local2.53ms - 2.63msunsure 🔍
-1% - +4%
-0.02ms - +0.10ms		-faster ✔
4% - 8%
0.10ms - 0.22ms
preact-hooks2.70ms - 2.78msslower ❌
6% - 10%
0.15ms - 0.26ms		slower ❌
4% - 9%
0.10ms - 0.22ms		-

usedJSHeapSize

VersionAvg timevs preact-mainvs preact-localvs preact-hooks
preact-main0.57ms - 0.58ms-unsure 🔍
-0% - +2%
-0.00ms - +0.01ms		faster ✔
2% - 4%
0.01ms - 0.02ms
preact-local0.56ms - 0.57msunsure 🔍
-2% - +0%
-0.01ms - +0.00ms		-faster ✔
3% - 4%
0.02ms - 0.02ms
preact-hooks0.59ms - 0.59msslower ❌
2% - 4%
0.01ms - 0.02ms		slower ❌
3% - 4%
0.02ms - 0.02ms		-
todo

duration

VersionAvg timevs preact-mainvs preact-localvs preact-hooks
preact-main26.16ms - 26.38ms-faster ✔
0% - 2%
0.04ms - 0.55ms		faster ✔
2% - 4%
0.64ms - 1.23ms
preact-local26.33ms - 26.79msslower ❌
0% - 2%
0.04ms - 0.55ms		-faster ✔
1% - 4%
0.28ms - 1.00ms
preact-hooks26.93ms - 27.48msslower ❌
2% - 5%
0.64ms - 1.23ms		slower ❌
1% - 4%
0.28ms - 1.00ms		-

usedJSHeapSize

VersionAvg timevs preact-mainvs preact-localvs preact-hooks
preact-main0.79ms - 0.79ms-unsure 🔍
-0% - -0%
-0.00ms - -0.00ms		faster ✔
3% - 3%
0.03ms - 0.03ms
preact-local0.79ms - 0.79msunsure 🔍
+0% - +0%
+0.00ms - +0.00ms		-faster ✔
3% - 3%
0.02ms - 0.02ms
preact-hooks0.82ms - 0.82msslower ❌
3% - 3%
0.03ms - 0.03ms		slower ❌
3% - 3%
0.02ms - 0.02ms		-

tachometer-reporter-action v2 for Benchmarks

@JoviDeCroock JoviDeCroock mentioned this pull request Sep 7, 2023
Merged
@github-actions
Copy link

github-actions bot commented Sep 7, 2023 •
edited
Loading

Size Change: +92 B (0%)

Total Size: 59.7 kB

Filename Size Change
dist/preact.js 4.5 kB +14 B (0%)
dist/preact.min.js 4.54 kB +15 B (0%)
dist/preact.min.module.js 4.54 kB +15 B (0%)
dist/preact.min.umd.js 4.57 kB +18 B (0%)
dist/preact.module.js 4.52 kB +15 B (0%)
dist/preact.umd.js 4.58 kB +15 B (0%)
ℹ️ View Unchanged
Filename Size Change
compat/dist/compat.js 3.98 kB 0 B
compat/dist/compat.module.js 3.9 kB 0 B
compat/dist/compat.umd.js 4.04 kB 0 B
debug/dist/debug.js 3.52 kB 0 B
debug/dist/debug.module.js 3.52 kB 0 B
debug/dist/debug.umd.js 3.6 kB 0 B
devtools/dist/devtools.js 232 B 0 B
devtools/dist/devtools.module.js 240 B 0 B
devtools/dist/devtools.umd.js 315 B 0 B
hooks/dist/hooks.js 1.52 kB 0 B
hooks/dist/hooks.module.js 1.56 kB 0 B
hooks/dist/hooks.umd.js 1.6 kB 0 B
jsx-runtime/dist/jsxRuntime.js 963 B 0 B
jsx-runtime/dist/jsxRuntime.module.js 938 B 0 B
jsx-runtime/dist/jsxRuntime.umd.js 1.04 kB 0 B
test-utils/dist/testUtils.js 453 B 0 B
test-utils/dist/testUtils.module.js 454 B 0 B
test-utils/dist/testUtils.umd.js 536 B 0 B

compressed-size-action

marvinhagemeister
marvinhagemeister approved these changes Sep 8, 2023
View reviewed changes
Copy link
Member

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

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

Excited to have this!

@coveralls
Copy link

coveralls commented Sep 25, 2023 •
edited
Loading

Coverage Status

coverage: 99.467% (+0.003%) from 99.464%
when pulling b1c6ce5 on skip-comment-nodes
into da9d488 on main.

@JoviDeCroock
Copy link
Member Author

@marvinhagemeister will this benefit fresh, I haven't really been able to find good testbeds for the streaming renderer however I can merge this if it already benefits yall

@JoviDeCroock
Copy link
Member Author

JoviDeCroock commented Nov 10, 2023 •
edited
Loading

CC @andrewiggins I rebased this to work with your changes, would love a double check from your side

Also not sure whether it's worth to pursue this if we're not going for the streaming renderer

@JoviDeCroock JoviDeCroock merged commit 899e9d9 into main Dec 29, 2023
@JoviDeCroock JoviDeCroock deleted the skip-comment-nodes branch December 29, 2023 19:01
@JoviDeCroock JoviDeCroock mentioned this pull request Feb 3, 2024
Merged
@X-oss-byte X-oss-byte mentioned this pull request May 21, 2024
Open
@q1blue q1blue mentioned this pull request May 24, 2024
Open
@Leeze555 Leeze555 mentioned this pull request May 31, 2024
Open
@X-oss-byte X-oss-byte mentioned this pull request Jun 5, 2024
Open
@Jircs1 Jircs1 mentioned this pull request Jun 10, 2024
Open
This was referenced Jun 15, 2024
Open
Open
Open
@vAHiD55555 vAHiD55555 mentioned this pull request Jun 20, 2024
Open
@JoviDeCroock JoviDeCroock mentioned this pull request Jul 1, 2024
Closed
@X-oss-byte X-oss-byte mentioned this pull request Jul 22, 2024
Open
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@developit developit Awaiting requested review from developit

@andrewiggins andrewiggins Awaiting requested review from andrewiggins

@marvinhagemeister marvinhagemeister Awaiting requested review from marvinhagemeister

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@JoviDeCroock @coveralls @andrewiggins @marvinhagemeister