Radium: Performance concerns

[iclanzan]

Computing styles can be quite expensive with all the merging and mapping of objects and is made worse by the fact that they are recomputed on every render, even if they don’t change.

Any thoughts on how the situation can be improved?

[alexlande]

Warning: I don't have perfect answers for you on this one. :)

There are a few issues I think:

1. Yeah, computing styles can be expensive, particularly if you're using all of Radium's features together. We haven't seen any particular performance bottlenecks here yet in our own apps, but it is something to be aware of and that we're keeping an eye on.
2. I think there's ample room in style-resolver.js for optimization, which should help some. Any help there would be appreciated.
3. Sounds like you're suggesting caching resolved styles or some sort of check before re-calculating styles. I'm not sure how that would work exactly, but it does sound like an interesting direction. If you have any ideas please let me know, would love to talk about it.
4. On the plus side, the resulting styles are diffed and selectively applied to the DOM, so the only potential bottleneck is the actual resolution of the style object.

[colinmegill]

Seems like the first thing to try is to cache styleObj before this.buildStyles(styleObj) is invoked, check for equality, see if that shows promise at scale? +1 that the perf on the DOM side is significantly better on perf. Stealing a nice bit of thinking from Matt Zabriskie (@mzabriskie on Twitter) as he adds the following in a comment:

"With a stylesheet you will have selectors that contain declaration blocks. The browser will have to parse the stylesheet, then scan the document to apply the declarations with elements that match the selectors. The performance of this is affected by how complicated the selector is, and how large the document is. Not to mention anytime a reflow occurs this all has to be done all over again.

Using inline styles would avoid all of this processing. You are telling the browser that this element has these style rules, plain and simple. No cascading, no selector matching."

I'm hoping, @iclanzan, that we're already more performant on large apps with complex selectors, because Radium doesn't use selectors.

[colinmegill]

@alexlande upon waking up I played with an idea:

this.hasChanged(this.state.inputsToThisComponentsThatWouldTriggerAStyleRecompute) ? this.buildStyles(styles) : this.state.styles

I immediately invalidated this because we're not just talking about the inputs to the style recompute as data, we're talking about the styles themselves as data... data that requires listeners. Landed back where we were the other day re: stores as a possible perf solution for large projects. If the inputs have changed, trigger a recompute, and if the styles themselves have changed, trigger a recompute. Else, if same inputs and same styles, don't.

[alexlande]

I have some more thoughts about this, but where I'm landing right now:

1. Styles are based on props + state. Using the PureRenderMixin or similar would prevent costly recalculations because the component won't re-render. Would recommend this with immutable data on any applications where performance might be a bottleneck anyway.
2. We should be able to relatively easily cache the currently active modifiers, browser states, and media queries of a component (these are all shallow data structures) then do a shallow equality check on those and only get and merge styles if any have changed. That seems like the most sensible place to start to me. I can start putting together a PoC of that.
3. At this point, any expensive calculations taking place directly in a style object (that are based on a components props or state) are purely a user-land thing. You could optimize that based on your own use cases. (And I would hope you wouldn't have particularly expensive computation going on there anyway, most of the time.)

[iclanzan]

@alexlande your first idea would not help if some property changes that does not affect style. The PureRenderMixin would return true, which causes a render, which in turn causes the styles to recompute even if they haven’t changed.

The second idea I think is a good one and worth pursuing.

[axemclion]

Note from ReactJS workshops in Seattle

@colinmegill How about using something like browser-perf for automating the performance measurement ? I generated these graphs for bootstraps automatically, could try the same for Radium

[wmertens]

Looks like giving elements a class on creation is faster (and probably more memory efficient) than giving them inline styles: http://jsperf.com/classes-vs-inline-styles

Why can't radium auto-generate classes and apply those? Then :hover etc can be CSS effects instead of the brittle JS versions.

An Element would get uniquely-named classes, and per-instance changes could still be inline. If the API were changed a little so you can specify Element class-styles and then instance styles like usual, the former could be CSS classes with CSS :hover and the latter would be js-based.

You would get the best of both worlds: Only insert CSS for things you actually use, and calculated instance styles for perfect presentation, all located right where the element is coded.

[iclanzan]

@wmertens the performance difference is negligible and having states like hover and active computed in JS has more uses then just styling. In contrast, you can’t easily determine in JS if a CSS :hover state is being applied to an element.

[wmertens]

@iclanzan Hmmm I wouldn't call 20% negligible... And I'm not proposing to remove js-based :hover, just to use the CSS one if it's safe.

So basically I'm proposing a per-reactElement style that will always be applied to the element and lives in a autogenerated class, and the inline style can be done as usual, only including the differences from the class-based style.

[christla]

@wmertens - agree, something like JSS (http://slides.com/kof/jss#/) or RCSS (https://github.com/chenglou/RCSS)

[wmertens]

Indeed!

On Mon, Mar 9, 2015 at 1:46 AM christla [email protected] wrote:

@wmertens https://github.com/wmertens - agree, could be something like
JSS or RCSS are doing

—
Reply to this email directly or view it on GitHub
https://github.com/FormidableLabs/radium/issues/58#issuecomment-77785504
.

[silhouettes]

@wmertens With the jsperf testcase you posted, someone modified it to be a more representative comparison, in which case the difference is genuinely negligible:
http://jsperf.com/classes-vs-inline-styles/2

[wmertens]

Wow, on my phone inline styles as an object (the way react does it) are
faster than having a even only a single class!

http://jsperf.com/classes-vs-inline-styles/3 - I added a single combined
class since that would be the ideal of a separate calculated class for
given elements.

That alleviates my concerns about performance.

I was also worried about memory usage but I thought about it and I think
that inline styles don't use more memory than a class list since the result
is an element with calculated style attributes.

So, issue closed where I am concerned :) Thanks @silhouettes!
On Sat, May 16, 2015, 12:40 AM Richard Hua [email protected] wrote:

@wmertens https://github.com/wmertens With the jsperf testcase you
posted, someone modified it to be a more representative comparison, in
which case the difference is genuinely negligible:
http://jsperf.com/classes-vs-inline-styles/2

—
Reply to this email directly or view it on GitHub
https://github.com/FormidableLabs/radium/issues/58#issuecomment-102541552
.

[ianobermiller]

Closing unless we find some specific, measurable performance issues in Radium. If anyone disagrees, feel free to reopen.

[axemclion]

@ianobermiller Are there any specific test cases ? I could run them using something like browser-perf to see how inline styles vs classes impact at browser rendering times (like paints and layouts) and not just javascript time reported.

[ianobermiller]

We don't currently have any test cases, but feel free to come up with some. The only place I might be concerned about perf is if you were manually animating an element. For the most part, I don't believe it is or will be an issue.

[paulirish]

Yeah we need a more reasonable benchmark to test this.

JSperf runs things in a extremely non-realistic environment, so it's results concerning interaction with DOM, style, and CSS will be 95% irrelevant.