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: improve VNode creation performance with compiler hints #3334

Merged
merged 16 commits into from Jun 22, 2021
Merged

perf: improve VNode creation performance with compiler hints #3334

merged 16 commits into from Jun 22, 2021

Conversation

HcySunYang
Copy link
Member

@HcySunYang HcySunYang commented Mar 2, 2021

Background

The original idea came from @yyx990803, and I explored it.

We are currently doing a LOT of runtime checks in createVNode which is probably the most important perf hot spot. And these can be skipped by the compiler hints.

Differentiate element vs component vnodes

Add dedciated createElementVNode and createComponentVNode methods.

the createElementVNode can skip:

Both methods makes assumptions about pre-normalized inputs (listed below)

Props proxy check

We check isProxy(props) in case it's a user reactive object. For both createElementVNode and createComponentVNode this can be skipped by lifting this check to the generated code instead.

The guard code is only needed if a single v-bind is used:

<div v-bind="foo" />

output:

createElementVNode('div', guardReactiveProps(foo))

In other cases it's either an object literal, or mergeProps will be generated which already spreads the object.

Pre-normalize classes and styles

We currently check and normalize class and style when creating vnode, which can be done on-demand by compiling.

  1. Static class/style don't need normalization and can be generated as-is:
<div class="foo" />

output:

createElementVNode('div', { class: 'foo' })
  1. Dynamic class/style needs to be wrapped by nromalizeClass/normalizeStyle
<div :class="foo"  />

output:

createElementBlock("div", {
  class: normalizeClass(_ctx.foo)
})
  1. With single v-bind:
<div v-bind="obj" />

output:

createElementBlock("div", normalizeProps(guardReactiveProps(_ctx.obj)))

normalizeProps is used to normalize both class and style.

  1. Dynamic key binding, need to be wrapped by normalizeProps:
<div :[dynamic]="val" />

output:

createElementBlock("div", normalizeProps({ [_ctx.dynamic || ""]: _ctx.val }))

Pre-nomralize ShapeFlag

createElementVNode's shapeFlag can be full static analyzed and the shapeFlag check can be skipped:

createComponentVNode

  • Suspense & Teleport shape flags can be statically analyzed
  • Stateful vs. Functional still needs to be determined at runtime
  • Always call normalizechildren for components to be safe.

This should allow createElementVNode to be as efficient as possible.

@HcySunYang HcySunYang changed the title feat: improve VNode creation performance with compiler hints perf: improve VNode creation performance with compiler hints Mar 2, 2021
@yyx990803
Copy link
Member

yyx990803 commented Mar 2, 2021

  1. Static class appears before v-bind:
<p class="foo" v-bind="obj" />

Need full normalization, because v-bind can override the static class.

output:

createElementBlock("p", 
  normalizeProps(mergeProps({ class: "foo" }, _ctx.obj))
)

mergeProps already performs class and style normalization so normalizeProps here seems unnecessary.

Since mergeProps is only called when v-bind is used, I think it's ok to always perform class/style normalization in it instead of introducing exponential complexity in the compiler.

Also this means when v-bind is combined with dynamic :class or :style bindings, we can just rely on mergeProps instead of generating normalizeClass and normalizeStyle calls.

* - context.forSSR = true
* see `subTransform` in `ssrTransformCompoent.ts`
*/
forSSR?: boolean
Copy link
Member

Choose a reason for hiding this comment

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

The name can be a bit confusing here. Maybe something more explicit like ssrClientBranch?

@HcySunYang
Copy link
Member Author

mergeProps already performs class and style normalization so normalizeProps here seems unnecessary.

Alright, didn't notice it

@HcySunYang
Copy link
Member Author

HcySunYang commented Mar 3, 2021

I adjusted the PR and updated the initial proposal above.

About the ssrClientBranch, do you mean something like this when compiling for SSR:

context.ssr = true
context.ssrClientBranch = false // default 
context.ssrClientBranch = true  // when compiling the fallback branch

@yyx990803 yyx990803 added version: minor ❗ p4-important Priority 4: this fixes bugs that violate documented behavior, or significantly improves perf. labels Mar 26, 2021
@HcySunYang
Copy link
Member Author

This PR is backward compatible, but not forward compatible. This means that if a third-party library is built with this PR, then this library cannot be used in the lower version of the Vue runtime. But it does not affect the user to use the old library

@yyx990803
Copy link
Member

I think this is worth it - we probably will have to introduce a "semi breaking change" in 3.1 where pre-compiled components using version >=3.1.0 will not work with runtime <3.1.0.

Btw it needs to be rebased now

@HcySunYang
Copy link
Member Author

Should be fine now

@yyx990803 yyx990803 changed the base branch from master to 3.2 June 22, 2021 19:44
@yyx990803
Copy link
Member

Note: I noticed that we still have mostly duplicated logic in createVNode and createComponentVNode, while at the same time we end up nesting several function calls in order to reuse other logic.

Since the gain in createComponentVNode will be rather minimal, createElementVNode seems to be the bigger win here. To simplify it, I merged createElementVNode with createPlainVNode into createBaseVNode. This is reused inside createComponentVNode while at the same time exported as createElementVNode.

Did a simple benchmark and createElementVNode is ~2.5 times faster than original createVNode at creating vnodes!

I also noticed a downside of the pre-normalized shape flags, as they result in a lot of extra null placeholders:

// before
createVNode('div', null, 'foo')

// after
createElementVNode('div', null, 'foo', null, null, 9 /* shapeFlag */)

I tested it in a medium sized project and the app.js chunk increased by 2.2%.

Since we are already separating element vs. component vnodes, we know that for compiled element vnodes they can only have text or array children. So I added a simpler children shapeFlag check for createElementVNode to only check if (children) and if (isString(children)) - and as expected, the benchmark result is almost the same. This allows us to remove the pre-normalization of shapeFlags from the compiler and reclaim the bundle size without sacrificing much runtime performance.

@flozero
Copy link

flozero commented Feb 16, 2022

Hello thank you a lot for the work here.

It will sounds like a dumb questions...

So with this PR we can't build anymore universal component library with vue demi right for vue2 and vue3 ?

Because unfortunately there is still a lot of companies that keep using vue2 and not already started the transition that would want to build vue3 design system without having to rewrite it after when they finally transition to vue3.

Can we have a clear explanations / documentations about how to achieve that and where this break. I understand there is a lot of work to do but if you want companies to follow we need a way more better documentation / support on that.

Even after looking at existing ui libs no ones is doing this kind of transition. It's build vue3 right now or stay with the vue2.

And for most of them none of them have finished the transition...

I hope this message will be listened. Have a great weekend

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
❗ p4-important Priority 4: this fixes bugs that violate documented behavior, or significantly improves perf. version: minor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants