Dashboard Performance: HtmlContent improvements #4726
Conversation
| return ( | ||
| <div | ||
| {...props} | ||
| dangerouslySetInnerHTML={{ __html: sanitize(children) }} // eslint-disable-line react/no-danger | ||
| /> | ||
| ); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Have you tried to use useMemo instead of React.memo? React.memo changes behavior of component (due to shallow comparison of props, component may not properly react to props changes):
const html = useMemo(() => ({ __html: sanitize(children) }), [children]);
return <div {...props} dangerouslySetInnerHTML={html} />There was a problem hiding this comment.
Yes, useMemo should have the same performance in concept, but not re-rendering the component at all looked better. For the HtmlContent component it should be totally dependent on its props. I quickly checked its usages, it has only an string for className (basic variable types don't create issues with shallow comparison) and the children node didn't seem a problem as well.
There was a problem hiding this comment.
Thanks for explanation! While I personally prefer useMemo, React.memo looks good as well 👍
| </HtmlContent> | ||
| {!isEmpty(widget.getQuery().description) && ( | ||
| <HtmlContent className="text-muted markdown query--description"> | ||
| {markdown.toHTML(widget.getQuery().description || "")} |
There was a problem hiding this comment.
How about markdown.toHTML performance? Is it fast enough, or maybe also needs some caching?
There was a problem hiding this comment.
I didn't cache it with useMemo because it runs widget.getQuery() currently, it would need to have the widget as a dependency. Since that component is basically re-rendered when widgetchanges, that didn't seem that relevant 🤔. Also now it only runs that function when the query has a description.
What type of PR is this? (check all applicable)
Description
Second PR for #4714, basically all widgets had this component rendered unnecessarily (also it could be memoized as its props should always return the same component). Although the
sanitizefunction is kinda of performant, I believe it scales with the number of renders and widgets.Individual improvement (based on master)
(The performance analysis were run with the same time for each dashboard twice (with the least value selected) to make sure results more reliable.)
The Scripting time I'm noting actually varies considerably, but when there's an actual improvement, the minimum value you can get diminishes, so it's been the best benchmark I've found so far. It just makes it a bit more difficult to measure dashboards with low avg scripting time (< 3s).
Related Tickets & Documents
#4714
Mobile & Desktop Screenshots/Recordings (if there are UI changes)
--