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

Feat/proxy performance #1247

Closed
wants to merge 8 commits into from
Closed

Feat/proxy performance #1247

wants to merge 8 commits into from

Conversation

basvanmeurs
Copy link
Contributor

As mentioned in #1227, variable access from templates is a major hotspot in terms of performance. This PR reduces up to 80% of the variable access time. It does so by creating ultra-fast and optimizable getter functions.

The peformance difference can be seen here. Notice that the left side is with the changes in this PR, right side is the current Vue implementation. The 'get' time is much lower (70 vs 420) and overall it causes a performance improvement of about 10%.

performance

In terms of memory usage, getter functions must be created per instance. Initial tests proved that it seems to has little effect on the memory usage.

…oped functions that return values faster

It's much faster when repeatedly accessing variables, but we still need to investigate the memory implications when having a large number of component instances
@basvanmeurs
Copy link
Contributor Author

Please don't merge this pull request yet. I'd like to do some further analysis on the performance of creating new component instances, memory usage and optimizer behavior.

@pikax
Copy link
Member

pikax commented May 28, 2020

Please don't merge this pull request yet

You mark it as WIP or move it to draft

@posva posva marked this pull request as draft May 28, 2020 09:20
…ponent instance

Because it does not impact proxy get performance and it does decrease memory consumption and improve instance creation performance.
… rather reuse them per component type

Reusing ensures that memory usage is limited, while only slightly impacting performance.
@basvanmeurs basvanmeurs marked this pull request as ready for review May 29, 2020 13:20
@basvanmeurs
Copy link
Contributor Author

basvanmeurs commented May 29, 2020

I changed some things in the original idea and did some extensive performance testing, comparing the current vue-next version with this PR. The results are further below, but most importantly they show that instance proxy performance in component instance creation has become about 15% slower (because some functions and function arguments need to be put into the propGetters cache), while setup access (particles test) has become about 75% faster. It only makes a slight impact on the total time, although in some cases (when there are many v-for items or components, and many setup accesses), there is certainly a performance improvement with this PR.

In general, nothing beats exposing the setup state (as raw object) directly and changing the syntax to setup.myvar.value and props.myprop instead of accepting just myvar and props in the template. This gives the best possible performance, makes it clear immediately where the property is coming from and is closest to writing your own render functions.

Nevertheless, I realise that we want to preserve the existing syntax. So for that case this PR can help limiting the performance difference somewhat. Still, I would prefer to see 'setup' and 'prop' being directly available, as mentioned by @RobbinBaauw in #1227.

100 new components per frame, 200 frames
Previous flushJobs: 1194ms, 1236ms, 1213ms
Previous proxy get: 287ms, 262m, 308ms
New flushJobs: 1115ms, 1171ms, 1161ms
New proxy get: 325ms, 335ms, 316ms

100 reused components per frame, 200 frames
Previous flushJobs: 862ms, 1038ms, 720ms
Previous proxy get: 155ms, 175ms, 135ms
New flushJobs: 786ms, 991ms, 937ms
New proxy get: 136ms, 101ms, 124ms

Particles test (6000 updated v-for items within the same component, all requesting 1 setup ref):
Previous flushJobs:
1905, 1893, 1864
Previous proxy get:
376, 405, 410

New flushJobs:
1759, 1643, 1725
New proxy get:
88, 103, 102

@ZhangJian-3ti
Copy link
Contributor

Based on the data you provide, seems not worthy in the real world cases.

@yyx990803
Copy link
Member

First of all, thanks a lot of coming up with the idea and experimenting with this. Unfortunately, the gain here doesn't seem to justify the increased complexity of the implementation.

I still think this is an area worth exploring - but I think a more compiler-informed approach would be more plausible.

@yyx990803 yyx990803 closed this Jun 25, 2020
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