swordev / suid

A port of Material-UI (MUI) built with SolidJS.
https://suid.io
MIT License
660 stars 47 forks source link

Rendering performance questions around `createStyled` #258

Open Bersaelor opened 11 months ago

Bersaelor commented 11 months ago

Hey there,

so recently I've been wondering about the performance of suid, after some questions on the solid discord, especially @fabiospampinato told me that rendering 25 cells shouldn't take 150ms on a M1 mac.

here is what I mean:

Screenshot 2023-08-24 at 15 44 30

when I explore the stack trace for opening a popup of 20 cells, It comes across createStyle from StyledComponent a lot.

Also, when reopening that popup a few times, the total rendering time goes down (related to StyleCache?)

Here's a stack-trace of the issue (Chrome export)

Now I've been using styled from mui in all my react applications and never noticed performance problems around it, but here in suid/solid it seems to be an issue?

This is happening on our production website https://app.letsrecast.ai/ , which has free registering, in case you want to generate performance reports to confirm the issue.

fabiospampinato commented 11 months ago

This is worth taking a closer look imo, because basically splitting/merging/spreading props seems expensive, and that's almost a key component of component libraries, so if those are slow the entire app may be slow as a result.

juanrgm commented 11 months ago

I investigated this issue here and I did some improvements here.

When I did the tests, I disabled the style system and that was not the problem.

SolidJS is very very fast when you use native components directly, but when you use nested custom components, SolidJS is "slow".

The other issue with SolidJS is that when you pass the properties to an inner component, each property is wrapper into a getter even though it has a primitive value:

function Component() {
  return <Subcomponent title={"static"} />
}

function Subcomponent(props) {
  return <div title={props.title} /> // { get title() { return props.title }
}

I'd like to find a solution to this, but no other project has run into these issues because they haven't gotten to that level of nesting.

For example, this is a very common pattern:

<ListItem>
  <ListItemButton>
    <ListItemText>text</ListItemText>
  </ListItemButton>
</ListItem>

and internally this is the route:

ListItem > ListItemStyled > li > ListItemButton > ListItemButtonStyled > div > ListItemText > ListItemTextStyled > div > span

Bersaelor commented 11 months ago

Thank you for looking into this @juanrgm . I already noticed your work on mergeProps when trying to understand why the suid powered website is so slow.

I just did a test and an example public detail page of our site spends 2.4s just in Script Evaluation ... I could replace all the suid-props with classic css (or tailwind etc) but I have to say I really like the convenience of using styled components, developing sites with the toolbox from MUI/Suid is so much faster then without.

ryansolid commented 10 months ago

This is interesting. Id love to get to the root cause. A few extra getters I doubt is the problem here as most reactive libraries memoize at each level which significantly more expensive. Getters while more expensive than function calls cost nearly nothing. Nesting in itself is not expensive. It could be related to mergeProps which happens when you spread. More often then not when faced with similar issue in the past its something upstream that isn't a memowhen it should be. But I haven't likely seen the same level of nesting. Probably since Im a bit adverse to it, knowing that it loses alot of the benefits of fine grained when you get down to diffing props. That being said it shouldn't be different compared to other solutions, and certainly not noticeably so.

The trace has Dev and HMR on which adds extra wrappers to every component which are expensive so I can see that cost in terms of depth. It would be interesting to see this with dev: false. If I understand the trace one thing is clicked and then a ton of work is done. It might be helpful to understand the scenario better.

Bersaelor commented 9 months ago

Thank you for the detailed reply @ryansolid , so if I record the performance of the live website, which shouldn't be built with dev, I still have my M1 macbook taking 428ms to render a feed with 20 cells ( https://app.letsrecast.ai/latest was the url):

For understanding of the trace: (1) user clicks on the title of a collection on the home page, like latest (2) app navigates to the route ~/latest (3) 20 content items are fetched from the API, 20 content cells with text, image etc are layed out, looks like this:

Screenshot 2023-10-11 at 11 23 47

Screenshot 2023-10-11 at 11 17 04

I'm not really sure how to read the call tree or bottoms-up call tree with minification:

Screenshot 2023-10-11 at 11 19 19

Screenshot 2023-10-11 at 11 19 35