lumino icon indicating copy to clipboard operation
lumino copied to clipboard

Investigate using `Node.contains` to avoid layout reflow

Open krassowski opened this issue 2 years ago • 0 comments

This is an attempt to see if Node.contains solves https://github.com/jupyterlab/jupyterlab/issues/9757 as suggested in https://github.com/jupyterlab/jupyterlab/issues/9757#issuecomment-997357312:

Results are inconclusive.

While it avoids reflows due to moving mouse over menu items, If my understanding is correct, those are only problematic if you move mouse over items while the DOM is evolving (e.g. math is slowly rendering, after scrolling, etc) - otherwise reflow only happens once and consecutive transitions between menu items are smooth. Based on my local profiling it looks like we get mere 2% speed up for the tested notebook (below).

Remaining issues:

  • opening submenus (and menus in general) also causes calls getBoundingClientRect (calculating position of the menu); I don't see an easy way around this yet, but this is relatively harmless - each takes about 50ms: Screenshot from 2021-12-19 21-58-26
  • moving mouse over the menu bar sets the focus() which also triggers reflow and this one is very noticeable: Screenshot from 2021-12-19 21-57-23
  • even if we remove focus() call the following update() call triggers reflow with identical effect (there is some

I recall that when I profiled a very different environment of mine I saw lumino's ElementExt.hitTest as a substantial contributor to the issue, but it does not seem obvious anymore to me (but I have not tested in that environment, so maybe it is only a bottleneck for certain setups?).

Testing procedure

Test notebook: https://github.com/UCB-stat-159-s21/site/blob/main/Notes/tests.ipynb

I opened the test notebook from https://github.com/jupyterlab/jupyterlab/issues/9757.

wget https://raw.githubusercontent.com/UCB-stat-159-s21/site/main/Notes/tests.ipynb

I scrolled to the end and waited until math is rendered fully. I then checked how many elements are in the DOM:

document.querySelectorAll('*').length
35909

(should be the same order of magnitude, might vary slightly depending on open sidebars/menu/files items, etc)

Some more notes:

  • [x] currElem!.textContent = newVNode.content; needs to have a check because virtual dom updates the actual DOM even if text has not changed and it is recorded by Chromium; it is not clear if it has a significant performance impact but we can likely cut some milliseconds here
  • It looks like its the requestAnimationFrame in messaging's schedule() which causes the "Recalculate Styles" task; there is a bug report for Chromium which might be related RequestAnimationFrame will trigger recalculate style even nothing changes: Screenshot from 2021-12-20 19-07-57
  • in said animation frame the classes of icons are updated which takes more time than it should considering that there are absolutely no icons in the menu I am testing (only empty placeholder icons nodes for each item): Screenshot from 2021-12-20 19-09-19

krassowski avatar Dec 19 '21 22:12 krassowski