Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

perf(reactivity): refactor effects #9480

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

Conversation

Simon-He95
Copy link
Contributor

No description provided.

@pikax
Copy link
Member

pikax commented Oct 26, 2023

I don't think this is a performance improvement, you not running 2 loops but you creating a new array, push to it and then looping it, not sure how this will affect the performance, I'd recommend benchmarking to prove there's actually performance benefits here.

@pikax pikax added need more info Further information is requested scope: compiler labels Oct 26, 2023
@Simon-He95
Copy link
Contributor Author

I don't think this is a performance improvement, you not running 2 loops but you creating a new array, push to it and then looping it, not sure how this will affect the performance, I'd recommend benchmarking to prove there's actually performance benefits here.我不认为这是性能改进,您没有运行 2 个循环,而是创建一个新数组,推送到它然后循环它,不确定这将如何影响性能,我建议进行基准测试以证明这里确实有性能优势。

Its execution will definitely be faster than before, because the previous logic is to traverse the entire data again, and each time it has to judge effect.computed, and an array is added so that the data that has been filtered in the first loop can be directly Loop execution, the amount of data you have the second time will be less, and no conditional judgment is needed anymore.

https://perf.link/%EF%BC%89%EF%BC%9AJSPerf#eyJpZCI6ImFvejkyNjFqa3kyIiwidGl0bGUiOiJGaW5kaW5nIG51bWJlcnMgaW4gYW4gYXJyYXkgb2YgMTAwMCIsImJlZm9yZSI6ImNvbnN0IGVmZmVjdHMgPSBbXVxuZm9yKGxldCBpPTA7aTwxMDA7aSsrKXtcbiAgZWZmZWN0cy5wdXNoKHtcbiAgIGNvbXB1dGVkOiBNYXRoLnJhbmRvbSgpPD0wLjVcbiAgfSlcbn1cbmNvbnN0IHRyaWdnZXJFZmZlY3QgPSAoKT0%2Be30iLCJ0ZXN0cyI6W3sibmFtZSI6IkZpbmQgaXRlbSAxMDAiLCJjb2RlIjoiICBjb25zdCBub3RDb21wdXRlZEVmZmVjdCA9IFtdXG4gIGZvciAoY29uc3QgZWZmZWN0IG9mIGVmZmVjdHMpIHtcbiAgICBpZiAoZWZmZWN0LmNvbXB1dGVkKSB7XG4gICAgICB0cmlnZ2VyRWZmZWN0KGVmZmVjdClcbiAgICB9IGVsc2Uge1xuICAgICAgbm90Q29tcHV0ZWRFZmZlY3QucHVzaChlZmZlY3QpXG4gICAgfVxuICB9XG4gIG5vdENvbXB1dGVkRWZmZWN0LmZvckVhY2goZWZmZWN0ID0%2BXG4gICAgdHJpZ2dlckVmZmVjdChlZmZlY3QpXG4gICkiLCJydW5zIjpbOTIwMDAsMjAwMCw1MDAwLDgwMDAwLDYxMDAwLDI1NjAwMCwyMTMwMDAsMzY3MDAwLDI1MzAwMCwzMTQwMDAsMjQ4MDAwLDM0MzAwMCw1MDAwLDIwMDAwLDEwNDAwMCw3MDAwLDEzMDAwLDI4MzAwMCwyMTAwMDAsMjg1MDAwLDI2NDAwMCw3MDAwLDgwMDAsNzQwMDAsMTIwMDAsMTc1MDAwLDM1MDAwMCwxMDAwLDMzMDAwMCwzMjAwMCw1MTAwMCwxNTMwMDAsMTk1MDAwLDI3MDAwMCwxODcwMDAsNDAwMCwzMTIwMDAsMjkzMDAwLDE1MDAwLDY3MDAwLDI1MDAwLDQwMDAwMCwzMjIwMDAsMjQ0MDAwLDM1MDAwMCwxMDEwMDAsMzY1MDAwLDM0MDAwLDE4OTAwMCw1MjAwMCw4MDAwLDI0MDAwLDEwMDAwLDE1MDAwLDEyODAwMCwzMjkwMDAsMTE3MDAwLDQwMDAsNjUwMDAsMjUwMDAsNzAwMCwyMzAwMCwxMDAwMCwxNTAwMCw1MDAwLDM1MDAwMCw0OTAwMCwxNjAwMCwzMTYwMDAsMCwxOTAwMCw1MDAwLDMwMDAwLDUwMDAsNzAwMCwyNzIwMDAsMTUzMDAwLDI3NzAwMCwyMDAwLDI1MDAwLDI1OTAwMCwyMjAwMDAsMjUwMDAwLDY0MDAwLDIyNzAwMCwzMjUwMDAsMjIzMDAwLDgwMDAsNjAwMDAsNDIwMDAsMzYwMDAsMTcwMDAsNTAwMCwxMDAwLDEwMDAsNjAwMDAsNTUwMDAsMTU3MDAwLDE1MTAwMCwyNTAwMF0sIm9wcyI6MTI1NDAwfSx7Im5hbWUiOiJGaW5kIGl0ZW0gMjAwIiwiY29kZSI6ImZvciAoY29uc3QgZWZmZWN0IG9mIGVmZmVjdHMpIHtcbiAgICBpZiAoZWZmZWN0LmNvbXB1dGVkKSB7XG4gICAgICB0cmlnZ2VyRWZmZWN0KGVmZmVjdClcbiAgICB9XG4gIH1cbiAgZm9yIChjb25zdCBlZmZlY3Qgb2YgZWZmZWN0cykge1xuICAgIGlmICghZWZmZWN0LmNvbXB1dGVkKSB7XG4gICAgICB0cmlnZ2VyRWZmZWN0KGVmZmVjdClcbiAgICB9XG4gIH1cbiIsInJ1bnMiOls2MDAwMCw1MDAwLDUwMDAsMTA0MDAwLDQ2MDAwLDEzMDAwLDU4MDAwLDIzMDAwMCwxMDAwLDIyNzAwMCwxNDEwMDAsMjI2MDAwLDUwMDAsNDAwMCw0MjAwMCwxMDAwLDIyMDAwMCwyMDAwMDAsMTM1MDAwLDE3NTAwMCwxNzMwMDAsMjEwMDAwLDMwMDAsMzkwMDAsMTUwMDAsOTQwMDAsMjMwMDAwLDIyOTAwMCwyMDEwMDAsMzgwMDAsOTAwMDAsMTY0MDAwLDEyNTAwMCwxMzAwMCwxMDgwMDAsMTk1MDAwLDE5MTAwMCwxOTUwMDAsMjMwMDAwLDU2MDAwLDcwMDAsMjMwMDAwLDE5NTAwMCwxMjUwMDAsMjMwMDAwLDY4MDAwLDIzMDAwMCw1MDAwLDEyODAwMCwyMzAwMDAsNDUwMDAsMTkwMDAsMTE2MDAwLDIwMDAsMTAwMDAwLDIzMDAwMCw2MzAwMCwxODMwMDAsMTcwMDAsMTYwMDAsODAwMCwxMjAwMCwyMTAwMDAsMTEwMDAsMjMwMDAwLDIyMDAwMCwyMDAwMCw4MDAwLDE5MzAwMCwxNzAwMDAsNTAwMCwxMDAwLDEyMDAwLDIyODAwMCwyMzAwMDAsMTgxMDAwLDExMTAwMCwxODYwMDAsMTk4MDAwLDExMDAwLDE4ODAwMCwxNjEwMDAsMTUwMDAwLDE4MDAwLDIzMDAwMCwyMzAwMDAsMTI0MDAwLDUwMDAsODAwMCwyMTAwMCwxNzAwMCw1MDAwLDIyMzAwMCwxMDAwLDIwMDAsMjQwMDAsMjUwMDAsODkwMDAsMTE0MDAwLDUwMDBdLCJvcHMiOjEwNjIxMH1dLCJ1cGRhdGVkIjoiMjAyMy0xMC0yNlQwODowNzowOS4zMDZaIn0%3D

@pikax
Copy link
Member

pikax commented Oct 26, 2023

Thank you for the link, based on that you can optimise it even further see perf

@Simon-He95
Copy link
Contributor Author

Quite good, but you need to ensure that it is consistent with the original logic, and you need to execute the effect.computed effect first.

@pikax pikax removed the need more info Further information is requested label Oct 26, 2023
@skirtles-code
Copy link
Contributor

skirtles-code commented Oct 26, 2023

The title for this PR seems to be off. It doesn't seem to be related to compiler-sfc or defineOptions. It looks like you've copied the commit message from #9453.

@Simon-He95
Copy link
Contributor Author

The title for this PR seems to be off. [The title for this PR seems to be off.] It doesn't seem to be related to compiler-sfc or defineOptions. [It doesn't seem to be related to compiler-sfc or defineOptions.] It looks like you've copied the commit message from #9453 [#9453] . [It looks like you've copied the commit message from #9453.] 此 PR 的标题似乎已关闭。 [此 PR 的标题似乎已关闭。它似乎与 compiler-sfcdefineOptions 无关。[它似乎与编译器-sfc或defineOptions无关。看起来您已经从 #9453 [#9453] 复制了提交消息。[看起来您已经从 #9453 复制了提交消息。

Do you have any better suggestions? Because compared to the previous logic, it does skip the second loop of the entire effects elements that need to be judged not to be effect.computed.

@sxzz
Copy link
Member

sxzz commented Nov 8, 2023

Please update your PR title. Seems this PR is not related to compiler-sfc.

@Simon-He95 Simon-He95 changed the title perf(compiler-sfc): defineOptions avoid redundant conditional judgments perf(effect): defineOptions avoid redundant conditional judgments Nov 8, 2023
@sxzz sxzz changed the title perf(effect): defineOptions avoid redundant conditional judgments perf(effect): refactor effects Nov 9, 2023
@sxzz sxzz changed the title perf(effect): refactor effects perf(reactivity): refactor effects Nov 9, 2023
Copy link

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 86.3 kB (-106 B) 32.9 kB (-31 B) 29.7 kB (-70 B)
vue.global.prod.js 132 kB (-149 B) 49.6 kB (-48 B) 44.5 kB (-5 B)

Usages

Name Size Gzip Brotli
createApp 47.9 kB (-62 B) 18.9 kB (-15 B) 17.2 kB (-8 B)
createSSRApp 51.1 kB (-94 B) 20.1 kB (-32 B) 18.4 kB (-22 B)
defineCustomElement 50.3 kB (-62 B) 19.7 kB (-14 B) 18 kB (+28 B)
overall 61.3 kB (-62 B) 23.7 kB (-17 B) 21.6 kB (+26 B)

yyx990803 added a commit that referenced this pull request Feb 25, 2024
…list tracking (#10397)

Bug fixes
close #10236
close #10069

PRs made stale by this one
close #10290
close #10354
close #10189
close #9480
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Waiting
Development

Successfully merging this pull request may close these issues.

None yet

7 participants