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

Compiler always batch many attribute expression in one effect #2438

Open
rfvtgbzxc opened this issue Feb 28, 2025 · 2 comments
Open

Compiler always batch many attribute expression in one effect #2438

rfvtgbzxc opened this issue Feb 28, 2025 · 2 comments

Comments

@rfvtgbzxc
Copy link

rfvtgbzxc commented Feb 28, 2025

Describe the bug

Compiler for custom renderer always batch many attribute expression in one effect, which cause that any one of them update, the others update together.
Like this:
`
return (()=>{
const _el$ = _$createElement("container")
, _el$2 = _$createElement("container")
, _el$3 = _$createElement("sprite")
, _el$4 = _$createElement("sprite")
, _el$5 = _$createElement("sprite")
, _el$6 = _$createElement("container")
, _el$7 = _$createElement("sprite")
, _el$8 = _$createElement("sprite")
, _el$9 = _$createElement("sprite")
, _el$10 = _$createElement("container");

    _$insertNode(_el$, _el$2);
    _$insertNode(_el$, _el$10);
    _$setProp(_el$, "mask", mask);
    _$insert(_el$, mask, _el$2);
    _$insertNode(_el$2, _el$3);
    _$insertNode(_el$2, _el$4);
    _$insertNode(_el$2, _el$5);
    _$insertNode(_el$2, _el$6);
    _$setProp(_el$2, "interactiveChildren", false);
    _$setProp(_el$4, "y", 6);
    _$setProp(_el$5, "scale", {
        x: 1,
        y: -1
    });
    _$insertNode(_el$6, _el$7);
    _$insertNode(_el$6, _el$8);
    _$insertNode(_el$6, _el$9);
    _$setProp(_el$7, "x", 1);
    _$setProp(_el$8, "x", 1);
    _$setProp(_el$8, "y", 6);
    _$setProp(_el$9, "x", 1);
    _$setProp(_el$9, "scale", {
        x: 1,
        y: -1
    });

    _$insert(_el$10, ()=>props.children);
    _$insert(_el$, ()=>// @ts-ignore
    (()=>{
        const _el$11 = _$createElement("sprite");
        const _ref$ = clientArea;
        typeof _ref$ === "function" ? _$use(_ref$, _el$11) : clientArea = _el$11;
        _$setProp(_el$11, "eventMode", 'auto');
        _$effect(_p$=>{
            const _v$15 = clientWidth()
              , _v$16 = clientHeight()
              , _v$17 = new PIXI.Rectangle(0,0,clientWidth(),clientHeight());
            _v$15 !== _p$._v$15 && (_p$._v$15 = _$setProp(_el$11, "width", _v$15, _p$._v$15));
            _v$16 !== _p$._v$16 && (_p$._v$16 = _$setProp(_el$11, "height", _v$16, _p$._v$16));
            _v$17 !== _p$._v$17 && (_p$._v$17 = _$setProp(_el$11, "hitArea", _v$17, _p$._v$17));
            return _p$;
        }
        , {
            _v$15: undefined,
            _v$16: undefined,
            _v$17: undefined
        });
        return _el$11;
    }
    )(), null);

    _$effect(_p$=>{
        const _v$ = scrollBarVisible()
          , _v$2 = clientWidth() - 12
          , _v$3 = barBackHeaderTexture()
          , _v$4 = barBackBodyTexture()
          , _v$5 = clientHeight() - 2 * 6
          , _v$6 = barBackHeaderTexture()
          , _v$7 = clientHeight()
          , _v$8 = frontBarTop()
          , _v$9 = barFrontHeaderTexture()
          , _v$10 = barFrontBodyTexture()
          , _v$11 = frontBarHeight() - 2 * 6
          , _v$12 = barFrontHeaderTexture()
          , _v$13 = frontBarHeight()
          , _v$14 = -props.scrollTop;
        _v$ !== _p$._v$ && (_p$._v$ = _$setProp(_el$2, "visible", _v$, _p$._v$));
        _v$2 !== _p$._v$2 && (_p$._v$2 = _$setProp(_el$2, "x", _v$2, _p$._v$2));
        _v$3 !== _p$._v$3 && (_p$._v$3 = _$setProp(_el$3, "texture", _v$3, _p$._v$3));
        _v$4 !== _p$._v$4 && (_p$._v$4 = _$setProp(_el$4, "texture", _v$4, _p$._v$4));
        _v$5 !== _p$._v$5 && (_p$._v$5 = _$setProp(_el$4, "height", _v$5, _p$._v$5));
        _v$6 !== _p$._v$6 && (_p$._v$6 = _$setProp(_el$5, "texture", _v$6, _p$._v$6));
        _v$7 !== _p$._v$7 && (_p$._v$7 = _$setProp(_el$5, "y", _v$7, _p$._v$7));
        _v$8 !== _p$._v$8 && (_p$._v$8 = _$setProp(_el$6, "y", _v$8, _p$._v$8));
        _v$9 !== _p$._v$9 && (_p$._v$9 = _$setProp(_el$7, "texture", _v$9, _p$._v$9));
        _v$10 !== _p$._v$10 && (_p$._v$10 = _$setProp(_el$8, "texture", _v$10, _p$._v$10));
        _v$11 !== _p$._v$11 && (_p$._v$11 = _$setProp(_el$8, "height", _v$11, _p$._v$11));
        _v$12 !== _p$._v$12 && (_p$._v$12 = _$setProp(_el$9, "texture", _v$12, _p$._v$12));
        _v$13 !== _p$._v$13 && (_p$._v$13 = _$setProp(_el$9, "y", _v$13, _p$._v$13));
        _v$14 !== _p$._v$14 && (_p$._v$14 = _$setProp(_el$10, "y", _v$14, _p$._v$14));
        return _p$;
    }
    , {
        _v$: undefined,
        _v$2: undefined,
        _v$3: undefined,
        _v$4: undefined,
        _v$5: undefined,
        _v$6: undefined,
        _v$7: undefined,
        _v$8: undefined,
        _v$9: undefined,
        _v$10: undefined,
        _v$11: undefined,
        _v$12: undefined,
        _v$13: undefined,
        _v$14: undefined
    });
    return _el$;

} )();

`
When only -props.scrollTop change, _v$~_v$14 are called together. Some of them are costly, which makes system slow.

Your Example Website or App

https://playground.solidjs.com/anonymous/7df3bddd-51c0-4d9c-9fb4-0d9115269bbe

Steps to Reproduce the Bug or Issue

1.Visit the website palced
2.Check for the output file
3.Will shows the same result in the browser

Expected behavior

Is this a Agreement or Feature? How Can I improve about this?

Screenshots or Videos

No response

Platform

  • OS: [e.g. macOS, Windows, Linux]
  • Browser: [e.g. Chrome, Safari, Firefox]
  • Version: [e.g. 91.1]

Additional context

No response

@ryansolid
Copy link
Member

ryansolid commented Feb 28, 2025

Yeah this issue has been reported before. For the DOM it is highly beneficial to group things like this to prioritize creation over updates since updates are usually not expensive. The logic for the universal renderer is identical to that. The other difference I suppose is that the DOM only has simple values in props so the equality check is sufficient to shortcut things being called multiple times. Of course it is difficult to make that assumption across multiple environments either way. I'd love to get more feedback like this from other universal renderer users.

We generally recommend to createMemo around expensive operations which would avoid the cost here but obviously not having to worry about it is great. Then again if the compiler individually wraps everything that is a lot of overhead in some scenarios and the developer wouldn't be able to opt out in the way you can opt in.

@rfvtgbzxc
Copy link
Author

rfvtgbzxc commented Mar 1, 2025

Thank you! I'll try that method to avoid batching.

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

No branches or pull requests

2 participants