-
-
Notifications
You must be signed in to change notification settings - Fork 49
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
Rendering performance questions around createStyled
#258
Comments
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. |
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:
|
Thank you for looking into this @juanrgm . I just did a test and an example public detail page of our site spends 2.4s just in |
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. |
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: I'm not really sure how to read the call tree or bottoms-up call tree with minification: |
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:
when I explore the stack trace for opening a popup of 20 cells, It comes across
createStyle
fromStyledComponent
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.
The text was updated successfully, but these errors were encountered: