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

pref(runtime-vapor): refactor apiCreateIf #12629

Open
wants to merge 6 commits into
base: vapor
Choose a base branch
from

Conversation

edison1105
Copy link
Member

@edison1105 edison1105 commented Dec 30, 2024

  • for this template code
  <h1 v-if="msg">{{ msg }}</h1>
  <span v-else-if="foo">foo</span>
  <span v-else>bar</span>
  • current compiled code - call createIf twice, each v-else-if generates a new createIf call
  const n0 = _createIf(() => (_ctx.msg), () => {
    const n2 = t0()
    let _msg
    _renderEffect(() => _setText(n2, _ctx.msg))
    return n2
  }, () => _createIf(() => (_ctx.foo), () => {
    const n4 = t1()
    return n4
  }, () => {
    const n7 = t2()
    return n7
  }))
  • compiled code with this PR - call createIf only once, reduce overhead
  const n0 = _createIf(() => (_ctx.msg) ? () => {
    const n2 = t0()
    _renderEffect(() => _setText(n2, _ctx.msg))
    return n2
  } : _ctx.foo ? () => {
    const n4 = t1()
    return n4
  } : () => {
    const n7 = t2()
    return n7
  })

analysis

Initial Render

Before

  • Each createIf internally call renderEffect and create a new DynamicFragment instance

After

  • only one DynamicFragment instance
  • only one renderEffect call

Re-Render(Update)

Before

  • Nested createIf structure ensures only necessary conditions are evaluated
  • Each v-else-if branch:
    • Maintains its own DynamicFragment
    • Creates separate renderEffect for dynamic content
    • EffectScope stop/create when branches switch

After

  • All conditions are evaluated in sequence (seems like a drawback)
  • However, in reality:
    • Condition evaluation overhead is minimal (simple variable access or basic operations)
    • Thanks to short-circuit evaluation, not all conditions will be evaluated
    • Most importantly
      • only one DynamicFragment needs to be maintained
      • only one renderEffect

Trade-offs:

  • Added overhead:
    • May evaluate a few more condition expressions
    • This overhead is lightweight and immediate
  • Reduced overhead:
    • Less memory allocation
    • Fewer object creations

@edison1105 edison1105 marked this pull request as ready for review December 31, 2024 01:32
@edison1105 edison1105 added the scope: vapor related to vapor mode label Dec 31, 2024
@Doctor-wu
Copy link
Member

In the previous implementation, reactive data has an update boundary for each createIf, which is dropped in this PR (since we merge all reactive stuff into one createIf).

Considering this case

  <h1 v-if="msg">{{ msg }}</h1>
  <span v-else-if="foo">foo</span>
  <span v-else-if="foo1">foo1</span>
  <span v-else-if="foo2">foo2</span>
  <span v-else-if="foo3">foo3</span>
  <span v-else-if="foo4">foo4</span>
  <span v-else>bar</span>

If foo4 changed, we re-calc this template in the previous impl,

  <span v-else-if="foo4">foo4</span>
  <span v-else>bar</span>

but now we need re-calc the whole v-if-else chain in this PR.
We may need to do some bench on this.

@edison1105 edison1105 marked this pull request as draft December 31, 2024 02:12
@edison1105 edison1105 marked this pull request as ready for review December 31, 2024 08:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: vapor related to vapor mode
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants