FlexLayout icon indicating copy to clipboard operation
FlexLayout copied to clipboard

Remaining uses of getBoundingClientRect & performance concerns

Open LoganDark opened this issue 3 years ago • 8 comments

I've been looking into optimizing flexlayout and especially its integration into larger apps where calling getBoundingClientRect on every resize or every mouse movement creates abysmal performance.

updateLayoutMetrics and the measuring code is relatively harmless. This is the intended use of getBoundingClientRect and seems just fine to me. It doesn't show up in the profiler and isn't a culprit for poor performance.

However, there are some remaining uses of getDomRect and sometimes this.selfRef.current!.getBoundingClientRect() directly. I have tested an optimization where updateRect places the most recent client rect in this.selfRect and getDomRect simply reads from that field rather than forcing a synchronous recalculation.

That runs into an issue where the DOMRect returned by ResizeObserver does not actually contain position information, and cannot be used for obtaining the relative position of drag events which FlexLayout heavily relies on. There does not appear to be any other kind of observer that I can use to get the position of the layout without calling getBoundingClientRect.

It's possible that updating clientRect in a requestAnimationFrame callback could mean the browser doesn't have to recalculate anything because it already has. However I doubt it and have not tested it.

The weirdest use of getBoundingClientRect and arguably the hardest to remove (as there is no replacement for its functionality) is the one in TabButton.tsx where it is used for the tab drop and tab overflow logic. Both of these would stop working if that functionality was completely removed and there isn't an easy alternative. However it does affect performance in a significantly negative way despite its obscurity.

What are your thoughts? I see that you're already storing the tab rect in a field, that's good. It appears you have thought of this optimization before. Do you think it is possible to go further?

LoganDark avatar Aug 27 '21 23:08 LoganDark

Here's a good example of why getDomRect is absolutely evil:

image

This is during an animation.

LoganDark avatar Aug 28 '21 02:08 LoganDark

Can you show how to re-create the issue with a simple example.

nealus avatar Aug 30 '21 08:08 nealus

Can you show how to re-create the issue with a simple example.

This isn't really a simple problem. If you read the issue I'm showing concern about the remaining uses of getBoundingClientRect. To be perfectly clear, the only way to fix these performance problems in a large app with lots of styles is to remove getBoundingClientRect.

Please read the original post and let me know if you have any thoughts.

LoganDark avatar Aug 31 '21 04:08 LoganDark

Not a simple problem, but without a way to reproduce it, see where getBoundingClientRect is causing the issue, it's hard to modify the code to improve the performance. As you say, the tab overflow positioning relies on obtaining the tab rects so would be hard to change, but the getDomRect could, I think, be cached. However during normal (not realtime) resizing only the splitter div or outline rectangle are moved so there is no layout rendering to cause tab repainting.

nealus avatar Aug 31 '21 07:08 nealus

without a way to reproduce it, see where getBoundingClientRect is causing the issue, it's hard to modify the code to improve the performance.

Recalculate Style shows on Chrome's built-in profiler even on the regular demo - it's just that it doesn't take as long because the styles are less complex, i.e. the layout is the only thing on the page. It's entirely possible to try to optimize this, especially on the assumption that each Recalculate Style could result in a 150ms freeze like it does for me.

the getDomRect could, I think, be cached.

This is what I've been working on for the past couple days. It's hard to know when to call getBoundingClientRect again because there is no 'PositionObserver' (as much as I wish there was). So you would probably have to call it every frame. Maybe that's a better spot for it, I'm not sure.

However during normal (not realtime) resizing only the splitter div or outline rectangle are moved so there is no layout rendering to cause tab repainting.

The tab repainting happens every time my app re-renders, like the user switches pages or something. This is animated. But the animations become extremely choppy, because one of the pages happens to have a flexlayout, which calls getBoundingClientRect unconditionally on every single re-render.

I think the best solution would not be removing these calls per se, but finding a better way of deciding whether you need to recalculate or not

LoganDark avatar Aug 31 '21 08:08 LoganDark

ref 'positionobserver' this might work:

https://toruskit.com/blog/how-to-get-element-bounds-without-reflow/

https://developer.mozilla.org/en-US/docs/Web/API/Intersection_Observer_API

nealus avatar Aug 31 '21 08:08 nealus

also this may help: https://developer.mozilla.org/en-US/docs/Web/CSS/contain

nealus avatar Aug 31 '21 08:08 nealus

ref 'positionobserver' this might work:

https://toruskit.com/blog/how-to-get-element-bounds-without-reflow/

https://developer.mozilla.org/en-US/docs/Web/API/Intersection_Observer_API

That might actually be what I'm looking for, I did look at IntersectionObserver, I just didn't think of creating a new one each time to get the size. PR coming soon

LoganDark avatar Aug 31 '21 08:08 LoganDark