fix(child-diffing): avoid skipping re-orders #4088

JoviDeCroock · 2023-08-03T12:54:19Z

When we perform matchingIndex === i we risk skipping re-orders as we have a lot of cases where we reset skew to 0 for reverse re-ording. in inverse re-ordering we increase the skew by 1 hence we don't run into this issue, but we can't just decrement the skew to a negative number hence this safety 😅 this PR removes the hasMatchingIndex and only checks if we are currently on or next to the skew and appropriately invokes our move logic there.

Fixes #4087

github-actions · 2023-08-03T12:56:27Z

Size Change: -143 B (0%)

Total Size: 56.9 kB

Filename	Size	Change
`dist/preact.js`	4.33 kB	-21 B (0%)
`dist/preact.min.js`	4.35 kB	-25 B (0%)
`dist/preact.min.module.js`	4.35 kB	-25 B (0%)
`dist/preact.min.umd.js`	4.38 kB	-24 B (0%)
`dist/preact.module.js`	4.35 kB	-26 B (0%)
`dist/preact.umd.js`	4.39 kB	-22 B (0%)

ℹ️ View Unchanged

Filename	Size	Change
`compat/dist/compat.js`	3.91 kB	0 B
`compat/dist/compat.module.js`	3.83 kB	0 B
`compat/dist/compat.umd.js`	3.97 kB	0 B
`debug/dist/debug.js`	3.64 kB	0 B
`debug/dist/debug.module.js`	3.62 kB	0 B
`debug/dist/debug.umd.js`	3.71 kB	0 B
`devtools/dist/devtools.js`	232 B	0 B
`devtools/dist/devtools.module.js`	241 B	0 B
`devtools/dist/devtools.umd.js`	315 B	0 B
`hooks/dist/hooks.js`	1.53 kB	0 B
`hooks/dist/hooks.module.js`	1.56 kB	0 B
`hooks/dist/hooks.umd.js`	1.62 kB	0 B
`jsx-runtime/dist/jsxRuntime.js`	360 B	0 B
`jsx-runtime/dist/jsxRuntime.module.js`	326 B	0 B
`jsx-runtime/dist/jsxRuntime.umd.js`	441 B	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}

coveralls · 2023-08-03T12:59:08Z

coverage: 99.561%. remained the same when pulling a5754f4 on skew-based-fail into 7940137 on main.

marvinhagemeister

Great work 👍

This reverts commit 02ab119.

github-actions · 2023-08-03T14:24:44Z

📊 Tachometer Benchmark Results

Summary

duration

02_replace1k: unsure 🔍 -2% - +2% (-3.40ms - +2.69ms)
^{preact-local vs preact-main}
03_update10th1k_x16: unsure 🔍 -4% - +4% (-1.22ms - +1.53ms)
^{preact-local vs preact-main}
07_create10k: unsure 🔍 -1% - +1% (-7.07ms - +7.62ms)
^{preact-local vs preact-main}
filter_list: unsure 🔍 -4% - +3% (-1.22ms - +1.02ms)
^{preact-local vs preact-main}
hydrate1k: unsure 🔍 -1% - +1% (-0.64ms - +1.68ms)
^{preact-local vs preact-main}
many_updates: unsure 🔍 -3% - +3% (-0.92ms - +0.67ms)
^{preact-local vs preact-main}
text_update: unsure 🔍 -3% - +3% (-0.10ms - +0.10ms)
^{preact-local vs preact-main}
todo: unsure 🔍 -5% - +1% (-3.29ms - +0.49ms)
^{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.01ms - +0.00ms)
^{preact-local vs preact-main}
07_create10k: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
^{preact-local vs preact-main}
filter_list: unsure 🔍 -1% - +3% (-0.01ms - +0.04ms)
^{preact-local vs preact-main}
hydrate1k: unsure 🔍 -1% - +0% (-0.05ms - +0.02ms)
^{preact-local vs preact-main}
many_updates: unsure 🔍 -1% - +0% (-0.04ms - +0.01ms)
^{preact-local vs preact-main}
text_update: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
^{preact-local vs preact-main}
todo: unsure 🔍 -0% - +1% (-0.00ms - +0.01ms)
^{preact-local vs preact-main}

Results

02_replace1k

Browser: chrome-headless 115.0.5790.110
Sample size: 70
Built by: Benchmarks #1148
Commit: a5754f4

duration

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	157.44ms - 161.77ms	-	unsure 🔍 -2% - +2% -2.69ms - +3.40ms	unsure 🔍 -2% - +2% -2.98ms - +2.94ms
preact-local	157.11ms - 161.39ms	unsure 🔍 -2% - +2% -3.40ms - +2.69ms	-	unsure 🔍 -2% - +2% -3.33ms - +2.57ms
preact-hooks	157.60ms - 161.65ms	unsure 🔍 -2% - +2% -2.94ms - +2.98ms	unsure 🔍 -2% - +2% -2.57ms - +3.33ms	-

usedJSHeapSize

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	3.27ms - 3.28ms	-	unsure 🔍 -0% - +0% -0.01ms - +0.00ms	faster ✔ 0% - 1% 0.02ms - 0.03ms
preact-local	3.27ms - 3.28ms	unsure 🔍 -0% - +0% -0.00ms - +0.01ms	-	faster ✔ 0% - 1% 0.01ms - 0.03ms
preact-hooks	3.29ms - 3.30ms	slower ❌ 0% - 1% 0.02ms - 0.03ms	slower ❌ 0% - 1% 0.01ms - 0.03ms	-

run-warmup-0

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	51.79ms - 53.49ms	-	unsure 🔍 -4% - +1% -2.06ms - +0.73ms	faster ✔ 3% - 8% 1.33ms - 4.58ms
preact-local	52.20ms - 54.41ms	unsure 🔍 -1% - +4% -0.73ms - +2.06ms	-	faster ✔ 1% - 7% 0.52ms - 4.06ms
preact-hooks	54.22ms - 56.98ms	slower ❌ 2% - 9% 1.33ms - 4.58ms	slower ❌ 1% - 8% 0.52ms - 4.06ms	-

run-warmup-1

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	71.96ms - 73.98ms	-	unsure 🔍 -4% - +0% -2.93ms - +0.31ms	faster ✔ 2% - 6% 1.37ms - 4.90ms
preact-local	73.02ms - 75.54ms	unsure 🔍 -0% - +4% -0.31ms - +2.93ms	-	unsure 🔍 -5% - +0% -3.75ms - +0.09ms
preact-hooks	74.66ms - 77.56ms	slower ❌ 2% - 7% 1.37ms - 4.90ms	unsure 🔍 -0% - +5% -0.09ms - +3.75ms	-

run-warmup-2

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	53.50ms - 56.19ms	-	unsure 🔍 -6% - +1% -3.57ms - +0.33ms	unsure 🔍 -4% - +2% -2.48ms - +0.94ms
preact-local	55.05ms - 57.88ms	unsure 🔍 -1% - +7% -0.33ms - +3.57ms	-	unsure 🔍 -2% - +5% -0.91ms - +2.62ms
preact-hooks	54.55ms - 56.67ms	unsure 🔍 -2% - +5% -0.94ms - +2.48ms	unsure 🔍 -5% - +2% -2.62ms - +0.91ms	-

run-warmup-3

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	56.67ms - 60.49ms	-	unsure 🔍 -0% - +10% +0.03ms - +5.44ms	unsure 🔍 -6% - +2% -3.84ms - +1.44ms
preact-local	53.93ms - 57.76ms	faster ✔ 0% - 9% 0.03ms - 5.44ms	-	faster ✔ 2% - 11% 1.29ms - 6.58ms
preact-hooks	57.96ms - 61.60ms	unsure 🔍 -3% - +7% -1.44ms - +3.84ms	slower ❌ 2% - 12% 1.29ms - 6.58ms	-

run-warmup-4

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	61.93ms - 64.81ms	-	unsure 🔍 -3% - +4% -2.15ms - +2.25ms	unsure 🔍 -4% - +5% -2.40ms - +3.06ms
preact-local	61.66ms - 64.99ms	unsure 🔍 -4% - +3% -2.25ms - +2.15ms	-	unsure 🔍 -4% - +5% -2.57ms - +3.13ms
preact-hooks	60.72ms - 65.36ms	unsure 🔍 -5% - +4% -3.06ms - +2.40ms	unsure 🔍 -5% - +4% -3.13ms - +2.57ms	-

run-final

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	41.76ms - 43.91ms	-	unsure 🔍 -4% - +4% -1.71ms - +1.61ms	unsure 🔍 -5% - +2% -2.34ms - +0.82ms
preact-local	41.62ms - 44.15ms	unsure 🔍 -4% - +4% -1.61ms - +1.71ms	-	unsure 🔍 -6% - +2% -2.42ms - +1.01ms
preact-hooks	42.43ms - 44.75ms	unsure 🔍 -2% - +5% -0.82ms - +2.34ms	unsure 🔍 -2% - +6% -1.01ms - +2.42ms	-

03_update10th1k_x16

Browser: chrome-headless 115.0.5790.110
Sample size: 70
Built by: Benchmarks #1148
Commit: a5754f4

duration

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	33.54ms - 35.35ms	-	unsure 🔍 -4% - +4% -1.53ms - +1.22ms	unsure 🔍 -4% - +4% -1.37ms - +1.24ms
preact-local	33.56ms - 35.63ms	unsure 🔍 -4% - +4% -1.22ms - +1.53ms	-	unsure 🔍 -4% - +4% -1.31ms - +1.49ms
preact-hooks	33.57ms - 35.45ms	unsure 🔍 -4% - +4% -1.24ms - +1.37ms	unsure 🔍 -4% - +4% -1.49ms - +1.31ms	-

usedJSHeapSize

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	3.23ms - 3.24ms	-	unsure 🔍 -0% - +0% -0.00ms - +0.01ms	faster ✔ 0% - 1% 0.01ms - 0.03ms
preact-local	3.23ms - 3.24ms	unsure 🔍 -0% - +0% -0.01ms - +0.00ms	-	faster ✔ 1% - 1% 0.02ms - 0.03ms
preact-hooks	3.25ms - 3.26ms	slower ❌ 0% - 1% 0.01ms - 0.03ms	slower ❌ 1% - 1% 0.02ms - 0.03ms	-

07_create10k

Browser: chrome-headless 115.0.5790.110
Sample size: 50
Built by: Benchmarks #1148
Commit: a5754f4

duration

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	1291.54ms - 1301.94ms	-	unsure 🔍 -1% - +1% -7.62ms - +7.07ms	unsure 🔍 -1% - +0% -9.59ms - +4.16ms
preact-local	1291.82ms - 1302.20ms	unsure 🔍 -1% - +1% -7.07ms - +7.62ms	-	unsure 🔍 -1% - +0% -9.32ms - +4.43ms
preact-hooks	1294.95ms - 1303.95ms	unsure 🔍 -0% - +1% -4.16ms - +9.59ms	unsure 🔍 -0% - +1% -4.43ms - +9.32ms	-

usedJSHeapSize

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	25.32ms - 25.32ms	-	unsure 🔍 -0% - +0% -0.00ms - +0.00ms	unsure 🔍 -0% - -0% -0.02ms - -0.02ms
preact-local	25.32ms - 25.32ms	unsure 🔍 -0% - +0% -0.00ms - +0.00ms	-	unsure 🔍 -0% - -0% -0.02ms - -0.02ms
preact-hooks	25.34ms - 25.34ms	unsure 🔍 +0% - +0% +0.02ms - +0.02ms	unsure 🔍 +0% - +0% +0.02ms - +0.02ms	-

filter_list

Browser: chrome-headless 115.0.5790.110
Sample size: 60
Built by: Benchmarks #1148
Commit: a5754f4

duration

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	28.67ms - 30.24ms	-	unsure 🔍 -3% - +4% -1.02ms - +1.22ms	unsure 🔍 -2% - +5% -0.72ms - +1.37ms
preact-local	28.56ms - 30.15ms	unsure 🔍 -4% - +3% -1.22ms - +1.02ms	-	unsure 🔍 -3% - +4% -0.83ms - +1.28ms
preact-hooks	28.44ms - 29.82ms	unsure 🔍 -5% - +2% -1.37ms - +0.72ms	unsure 🔍 -4% - +3% -1.28ms - +0.83ms	-

usedJSHeapSize

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	1.43ms - 1.47ms	-	unsure 🔍 -3% - +1% -0.04ms - +0.01ms	unsure 🔍 -3% - +0% -0.04ms - +0.01ms
preact-local	1.44ms - 1.48ms	unsure 🔍 -1% - +3% -0.01ms - +0.04ms	-	unsure 🔍 -2% - +1% -0.03ms - +0.02ms
preact-hooks	1.45ms - 1.49ms	unsure 🔍 -0% - +3% -0.01ms - +0.04ms	unsure 🔍 -1% - +2% -0.02ms - +0.03ms	-

hydrate1k

Browser: chrome-headless 115.0.5790.110
Sample size: 50
Built by: Benchmarks #1148
Commit: a5754f4

duration

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	115.56ms - 117.21ms	-	unsure 🔍 -1% - +1% -1.68ms - +0.64ms	faster ✔ 0% - 2% 0.17ms - 2.80ms
preact-local	116.08ms - 117.72ms	unsure 🔍 -1% - +1% -0.64ms - +1.68ms	-	unsure 🔍 -2% - +0% -2.28ms - +0.34ms
preact-hooks	116.84ms - 118.90ms	slower ❌ 0% - 2% 0.17ms - 2.80ms	unsure 🔍 -0% - +2% -0.34ms - +2.28ms	-

usedJSHeapSize

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	5.95ms - 6.00ms	-	unsure 🔍 -0% - +1% -0.02ms - +0.05ms	unsure 🔍 -1% - +0% -0.05ms - +0.02ms
preact-local	5.95ms - 5.98ms	unsure 🔍 -1% - +0% -0.05ms - +0.02ms	-	unsure 🔍 -1% - +0% -0.05ms - +0.00ms
preact-hooks	5.97ms - 6.01ms	unsure 🔍 -0% - +1% -0.02ms - +0.05ms	unsure 🔍 -0% - +1% -0.00ms - +0.05ms	-

many_updates

Browser: chrome-headless 115.0.5790.110
Sample size: 50
Built by: Benchmarks #1148
Commit: a5754f4

duration

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	25.81ms - 26.97ms	-	unsure 🔍 -3% - +3% -0.67ms - +0.92ms	unsure 🔍 -4% - +2% -1.10ms - +0.42ms
preact-local	25.73ms - 26.81ms	unsure 🔍 -3% - +3% -0.92ms - +0.67ms	-	unsure 🔍 -4% - +1% -1.20ms - +0.27ms
preact-hooks	26.24ms - 27.23ms	unsure 🔍 -2% - +4% -0.42ms - +1.10ms	unsure 🔍 -1% - +5% -0.27ms - +1.20ms	-

usedJSHeapSize

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	4.49ms - 4.52ms	-	unsure 🔍 -0% - +1% -0.01ms - +0.04ms	unsure 🔍 -1% - +1% -0.02ms - +0.02ms
preact-local	4.47ms - 4.51ms	unsure 🔍 -1% - +0% -0.04ms - +0.01ms	-	unsure 🔍 -1% - +0% -0.04ms - +0.01ms
preact-hooks	4.49ms - 4.52ms	unsure 🔍 -1% - +1% -0.02ms - +0.02ms	unsure 🔍 -0% - +1% -0.01ms - +0.04ms	-

text_update

Browser: chrome-headless 115.0.5790.110
Sample size: 180
Built by: Benchmarks #1148
Commit: a5754f4

duration

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	3.42ms - 3.55ms	-	unsure 🔍 -3% - +3% -0.10ms - +0.10ms	faster ✔ 2% - 7% 0.06ms - 0.26ms
preact-local	3.40ms - 3.57ms	unsure 🔍 -3% - +3% -0.10ms - +0.10ms	-	faster ✔ 1% - 7% 0.05ms - 0.27ms
preact-hooks	3.57ms - 3.73ms	slower ❌ 2% - 8% 0.06ms - 0.26ms	slower ❌ 1% - 8% 0.05ms - 0.27ms	-

usedJSHeapSize

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	0.62ms - 0.62ms	-	unsure 🔍 -0% - +0% -0.00ms - +0.00ms	faster ✔ 9% - 11% 0.06ms - 0.07ms
preact-local	0.62ms - 0.62ms	unsure 🔍 -0% - +0% -0.00ms - +0.00ms	-	faster ✔ 9% - 11% 0.06ms - 0.07ms
preact-hooks	0.68ms - 0.69ms	slower ❌ 9% - 12% 0.06ms - 0.07ms	slower ❌ 9% - 12% 0.06ms - 0.07ms	-

todo

Browser: chrome-headless 115.0.5790.110
Sample size: 80
Built by: Benchmarks #1148
Commit: a5754f4

duration

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	59.94ms - 63.45ms	-	unsure 🔍 -1% - +5% -0.49ms - +3.29ms	unsure 🔍 -2% - +4% -1.48ms - +2.38ms
preact-local	59.59ms - 60.99ms	unsure 🔍 -5% - +1% -3.29ms - +0.49ms	-	unsure 🔍 -3% - +0% -2.01ms - +0.11ms
preact-hooks	60.44ms - 62.04ms	unsure 🔍 -4% - +2% -2.38ms - +1.48ms	unsure 🔍 -0% - +3% -0.11ms - +2.01ms	-

usedJSHeapSize

Version	Avg time	vs preact-main	vs preact-local	vs preact-hooks
preact-main	0.82ms - 0.82ms	-	unsure 🔍 -1% - +0% -0.01ms - +0.00ms	faster ✔ 2% - 3% 0.02ms - 0.02ms
preact-local	0.82ms - 0.83ms	unsure 🔍 -0% - +1% -0.00ms - +0.01ms	-	faster ✔ 2% - 3% 0.01ms - 0.02ms
preact-hooks	0.84ms - 0.84ms	slower ❌ 2% - 3% 0.02ms - 0.02ms	slower ❌ 2% - 3% 0.01ms - 0.02ms	-

_{tachometer-reporter-action v2 for Benchmarks}

JoviDeCroock requested review from andrewiggins and marvinhagemeister August 3, 2023 12:54

marvinhagemeister approved these changes Aug 3, 2023

View reviewed changes

JoviDeCroock added 5 commits August 3, 2023 16:24

add test

614e60b

fix

c892b58

remove unusued import

3d91a30

see if test is failing because of the tgz missing

385d4d7

Revert "see if test is failing because of the tgz missing"

a5754f4

This reverts commit 02ab119.

preactjs deleted a comment from github-actions bot Aug 3, 2023

JoviDeCroock force-pushed the skew-based-fail branch from 08c813e to a5754f4 Compare August 3, 2023 14:24

JoviDeCroock merged commit c294e8b into main Aug 3, 2023
13 checks passed

JoviDeCroock deleted the skew-based-fail branch August 3, 2023 14:33

JoviDeCroock mentioned this pull request Aug 12, 2023

10.17.0 #4094

Merged

JoviDeCroock mentioned this pull request Apr 27, 2024

backport 10.17 #4352

Draft

13 tasks

JoviDeCroock added a commit that referenced this pull request Apr 27, 2024

backport failing test for #4088

ef119b8

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(child-diffing): avoid skipping re-orders #4088

fix(child-diffing): avoid skipping re-orders #4088

JoviDeCroock commented Aug 3, 2023 •

edited

github-actions bot commented Aug 3, 2023 •

edited

coveralls commented Aug 3, 2023 •

edited

marvinhagemeister left a comment

github-actions bot commented Aug 3, 2023 •

edited

📊 Tachometer Benchmark Results

Summary

duration

usedJSHeapSize

Results

duration

usedJSHeapSize

run-warmup-0

run-warmup-1

run-warmup-2

run-warmup-3

run-warmup-4

run-final

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

fix(child-diffing): avoid skipping re-orders #4088

fix(child-diffing): avoid skipping re-orders #4088

Conversation

JoviDeCroock commented Aug 3, 2023 • edited

github-actions bot commented Aug 3, 2023 • edited

coveralls commented Aug 3, 2023 • edited

marvinhagemeister left a comment

Choose a reason for hiding this comment

github-actions bot commented Aug 3, 2023 • edited

📊 Tachometer Benchmark Results

Summary

duration

usedJSHeapSize

Results

duration

usedJSHeapSize

run-warmup-0

run-warmup-1

run-warmup-2

run-warmup-3

run-warmup-4

run-final

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

duration

usedJSHeapSize

JoviDeCroock commented Aug 3, 2023 •

edited

github-actions bot commented Aug 3, 2023 •

edited

coveralls commented Aug 3, 2023 •

edited

github-actions bot commented Aug 3, 2023 •

edited