Carbon: Tooltip performance issue with click listener

[chrisbygrave]

I'm using Tooltip components as part of field labels on a schema-generated form. Putting in a large schema I noticed rendering lag, which I tracked back to the Tooltip component, and in particular its use of the internal ClickListener. FYI this was with around 500 tooltips in total.

Tooltip is adding a mouse event listener in order to detect mouse clicks outside of an open tooltip in order to close the tooltip. However this ClickListener is created even when the tooltip isn't open, meaning I had 500 mouse event listeners active even without any open tooltips.

The most obvious fix here is to only render a ClickListener if the tooltip is open. A little local hacking showed this fixed the performance issue I was seeing, without any loss of functionality.

I can provide a PR tomorrow.

[emyarod]

related overflow menu issue for reference https://github.com/carbon-design-system/carbon/issues/5079 https://github.com/carbon-design-system/carbon/pull/5336

[chrisbygrave]

So, I implemented the above and it made me think I was off with my initial impression of where the time was being spent. I dug a little further, and oddly it seems to boil down to a performance improvement put in last year to wrap up calls to find the location on screen for floating menus inside requestAnimationFrame.

Here's the code running as is, rendering 1000 tooltips:

Replacing the requestAnimationFrame with a setTimeout:

Removing the requestAnimationFrame wrapper in componentDidMount in Tooltip:

I haven't included the call stack as there isn't anything Tooltip-related anywhere near the top of the list at this point.

@joshblack mentioning you here as you know best why we want the getBoundingRect calls to be async in the first place...

[asudoh]

Hi 👋 @chrisbygrave do you have your reduced case? Also having your saved profile data will be nice. Thanks!

[chrisbygrave]

I don't believe I'm changing the O complexity of the code. I just made the very small simple change noted in the comment.

https://github.com/carbon-design-system/carbon/blob/master/packages/react/src/components/Tooltip/Tooltip.js#L217

In the first performance measurement, I ran a page that rendered 1000 Tooltips and left the code unchanged.

In the second run I replaced the block above with:

    setTimeout(() => {
      this.getTriggerPosition();
    }, 0);

And in the final performance run I replaced it with:

    this.getTriggerPosition();

Nothing more complex than that.

My point is, the requestAnimationFrame call was put in for a reason, and I'm asking what that reason is, and what regressions we would all see if it was removed, as the performance implications are pretty large.

[asudoh]

It was because we tried to avoid synchronous layout query upon getBoundingClientRect() being part of React lifecycle. Anyhow, would you want to try #5881?

[chrisbygrave]

Many thanks! I tried with the latest in master and the performance issue is resolved: