Skip to content

refactor(children): remove redundant assignment to oldVNode#4956

Merged
rschristian merged 1 commit intopreactjs:mainfrom
ali-garajian:main
Nov 16, 2025
Merged

refactor(children): remove redundant assignment to oldVNode#4956
rschristian merged 1 commit intopreactjs:mainfrom
ali-garajian:main

Conversation

@ali-garajian
Copy link
Contributor

No description provided.

@github-actions
Copy link

github-actions bot commented Nov 9, 2025

📊 Tachometer Benchmark Results

Summary

duration

  • create10k: unsure 🔍 -0% - +1% (-3.24ms - +5.84ms)
    preact-local vs preact-main
  • filter-list: unsure 🔍 -0% - +0% (-0.07ms - +0.01ms)
    preact-local vs preact-main
  • hydrate1k: unsure 🔍 -2% - +1% (-1.42ms - +0.84ms)
    preact-local vs preact-main
  • many-updates: unsure 🔍 -3% - +0% (-0.45ms - +0.03ms)
    preact-local vs preact-main
  • replace1k: unsure 🔍 -1% - +1% (-0.78ms - +0.92ms)
    preact-local vs preact-main
  • text-update: unsure 🔍 -5% - +4% (-0.10ms - +0.08ms)
    preact-local vs preact-main
  • todo: unsure 🔍 -0% - +3% (-0.09ms - +0.80ms)
    preact-local vs preact-main
  • update10th1k: unsure 🔍 -4% - +3% (-1.19ms - +0.84ms)
    preact-local vs preact-main

usedJSHeapSize

  • create10k: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
    preact-local vs preact-main
  • filter-list: faster ✔ 0% - 1% (0.00ms - 0.01ms)
    preact-local vs preact-main
  • hydrate1k: faster ✔ 3% - 13% (0.17ms - 0.91ms)
    preact-local vs preact-main
  • many-updates: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
    preact-local vs preact-main
  • replace1k: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
    preact-local vs preact-main
  • text-update: unsure 🔍 -4% - +2% (-0.04ms - +0.02ms)
    preact-local vs preact-main
  • todo: slower ❌ 0% - 1% (0.01ms - 0.01ms)
    preact-local vs preact-main
  • update10th1k: unsure 🔍 -0% - +0% (-0.00ms - +0.00ms)
    preact-local vs preact-main

Results

create10k

duration

VersionAvg timevs preact-localvs preact-main
preact-local991.11ms - 997.53ms-unsure 🔍
-0% - +1%
-3.24ms - +5.84ms
preact-main989.80ms - 996.23msunsure 🔍
-1% - +0%
-5.84ms - +3.24ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local19.04ms - 19.04ms-unsure 🔍
-0% - +0%
-0.00ms - +0.00ms
preact-main19.04ms - 19.04msunsure 🔍
-0% - +0%
-0.00ms - +0.00ms
-
filter-list

duration

VersionAvg timevs preact-localvs preact-main
preact-local16.56ms - 16.60ms-unsure 🔍
-0% - +0%
-0.07ms - +0.01ms
preact-main16.58ms - 16.64msunsure 🔍
-0% - +0%
-0.01ms - +0.07ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local1.54ms - 1.54ms-faster ✔
0% - 1%
0.00ms - 0.01ms
preact-main1.54ms - 1.55msslower ❌
0% - 1%
0.00ms - 0.01ms
-
hydrate1k

duration

VersionAvg timevs preact-localvs preact-main
preact-local75.19ms - 76.66ms-unsure 🔍
-2% - +1%
-1.42ms - +0.84ms
preact-main75.35ms - 77.08msunsure 🔍
-1% - +2%
-0.84ms - +1.42ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local5.97ms - 6.51ms-faster ✔
3% - 13%
0.17ms - 0.91ms
preact-main6.52ms - 7.03msslower ❌
2% - 15%
0.17ms - 0.91ms
-
many-updates

duration

VersionAvg timevs preact-localvs preact-main
preact-local16.41ms - 16.57ms-unsure 🔍
-3% - +0%
-0.45ms - +0.03ms
preact-main16.47ms - 16.92msunsure 🔍
-0% - +3%
-0.03ms - +0.45ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local3.72ms - 3.72ms-unsure 🔍
-0% - +0%
-0.00ms - +0.00ms
preact-main3.72ms - 3.72msunsure 🔍
-0% - +0%
-0.00ms - +0.00ms
-
replace1k
  • Browser: chrome-headless
  • Sample size: 100
  • Built by: CI #5211
  • Commit: 3970255

duration

VersionAvg timevs preact-localvs preact-main
preact-local62.12ms - 63.17ms-unsure 🔍
-1% - +1%
-0.78ms - +0.92ms
preact-main61.90ms - 63.25msunsure 🔍
-1% - +1%
-0.92ms - +0.78ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local2.99ms - 2.99ms-unsure 🔍
-0% - +0%
-0.00ms - +0.00ms
preact-main2.99ms - 2.99msunsure 🔍
-0% - +0%
-0.00ms - +0.00ms
-

run-warmup-0

VersionAvg timevs preact-localvs preact-main
preact-local24.22ms - 24.86ms-unsure 🔍
-2% - +2%
-0.38ms - +0.49ms
preact-main24.19ms - 24.79msunsure 🔍
-2% - +2%
-0.49ms - +0.38ms
-

run-warmup-1

VersionAvg timevs preact-localvs preact-main
preact-local30.55ms - 31.38ms-unsure 🔍
-2% - +2%
-0.59ms - +0.64ms
preact-main30.49ms - 31.39msunsure 🔍
-2% - +2%
-0.64ms - +0.59ms
-

run-warmup-2

VersionAvg timevs preact-localvs preact-main
preact-local30.09ms - 30.80ms-unsure 🔍
-2% - +2%
-0.50ms - +0.47ms
preact-main30.13ms - 30.79msunsure 🔍
-2% - +2%
-0.47ms - +0.50ms
-

run-warmup-3

VersionAvg timevs preact-localvs preact-main
preact-local24.31ms - 24.51ms-faster ✔
0% - 2%
0.08ms - 0.60ms
preact-main24.51ms - 24.99msslower ❌
0% - 2%
0.08ms - 0.60ms
-

run-warmup-4

VersionAvg timevs preact-localvs preact-main
preact-local26.45ms - 27.97ms-unsure 🔍
-1% - +8%
-0.12ms - +2.02ms
preact-main25.51ms - 27.02msunsure 🔍
-7% - +0%
-2.02ms - +0.12ms
-

run-final

VersionAvg timevs preact-localvs preact-main
preact-local20.27ms - 20.87ms-unsure 🔍
-1% - +3%
-0.17ms - +0.64ms
preact-main20.06ms - 20.61msunsure 🔍
-3% - +1%
-0.64ms - +0.17ms
-
text-update

duration

VersionAvg timevs preact-localvs preact-main
preact-local1.97ms - 2.11ms-unsure 🔍
-5% - +4%
-0.10ms - +0.08ms
preact-main1.99ms - 2.10msunsure 🔍
-4% - +5%
-0.08ms - +0.10ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local0.97ms - 1.01ms-unsure 🔍
-4% - +2%
-0.04ms - +0.02ms
preact-main0.97ms - 1.03msunsure 🔍
-2% - +4%
-0.02ms - +0.04ms
-
todo

duration

VersionAvg timevs preact-localvs preact-main
preact-local31.65ms - 32.42ms-unsure 🔍
-0% - +3%
-0.09ms - +0.80ms
preact-main31.46ms - 31.91msunsure 🔍
-2% - +0%
-0.80ms - +0.09ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local1.26ms - 1.26ms-slower ❌
0% - 1%
0.01ms - 0.01ms
preact-main1.26ms - 1.26msfaster ✔
0% - 1%
0.01ms - 0.01ms
-
update10th1k

duration

VersionAvg timevs preact-localvs preact-main
preact-local32.74ms - 34.14ms-unsure 🔍
-4% - +3%
-1.19ms - +0.84ms
preact-main32.88ms - 34.35msunsure 🔍
-3% - +4%
-0.84ms - +1.19ms
-

usedJSHeapSize

VersionAvg timevs preact-localvs preact-main
preact-local2.93ms - 2.94ms-unsure 🔍
-0% - +0%
-0.00ms - +0.00ms
preact-main2.93ms - 2.94msunsure 🔍
-0% - +0%
-0.00ms - +0.00ms
-

tachometer-reporter-action v2 for CI

@coveralls
Copy link

coveralls commented Nov 9, 2025

Coverage Status

coverage: 99.442%. remained the same
when pulling 3970255 on ali-garajian:main
into eee0c6e on preactjs:main.

@rschristian
Copy link
Member

This one I'm less sure of, looks a little suspect looking at line 253/254 & this commit: b4a1cc2

Also, rebasing before opening PRs is generally appreciated. This has 6 commits associated with it because you haven't.

@ali-garajian
Copy link
Contributor Author

ali-garajian commented Nov 10, 2025

@rschristian sorry about the extra commits, won't happen again

I can see your concern, but I'm going through every branch of code after that assignment and it seems everything would be fine without it.
besides, the tests are passing

@rschristian
Copy link
Member

sorry about the extra commits, won't happen again

No worries, and you can rebase this yet. Just makes it a bit easier for us is all.

but I'm going to every branch of code after that assignment and it seems everything would be fine without it.

Looks a bit suspect to me yet, would need to look into it more later this week

besides, the tests are passing

In a perfect world that'd be enough, but in reality, test suites can have gaps. That's certainly a good sign though.

@rschristian rschristian merged commit 50fcda6 into preactjs:main Nov 16, 2025
13 checks passed
JoviDeCroock pushed a commit that referenced this pull request Nov 29, 2025
JoviDeCroock added a commit that referenced this pull request Nov 29, 2025
* refactor(children): remove redundant assignment to childVNode (#4951)

* refactor(children): remove redundant assignment to childVNode

* refactor(children): use key instead of childVNode.key

---------

Co-authored-by: Ali Garajian <[email protected]>

* refactor(diff): no need to reset props and context when creating component (#4954)

* refactor(children): remove redundant assignment to childVNode

* refactor(children): use key instead of childVNode.key

* refactor(diff): no need to reset props and context when creating component

---------

Co-authored-by: Ali Garajian <[email protected]>

* refactor(children): remove redundant assignment to oldVNode (#4956)

Co-authored-by: Ali Garajian <[email protected]>

* refactor(diff): set oldProps default value on declaration (#4959)

---------

Co-authored-by: Ali Garajian <[email protected]>
Co-authored-by: Ali Garajian <[email protected]>
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.

4 participants