Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sluggish UI when long, math-heavy notebooks are open. #9757

Closed
fperez opened this issue Feb 7, 2021 · 28 comments · Fixed by #13159
Closed

Sluggish UI when long, math-heavy notebooks are open. #9757

fperez opened this issue Feb 7, 2021 · 28 comments · Fixed by #13159

Comments

@fperez
Copy link
Contributor

fperez commented Feb 7, 2021

Description

We're noticing the UI in Lab 3.0.x (x ~ 5-7) getting sluggish (slow menu switching, latency when highlighting menu entries or typing, tab switching) when notebooks that are both long and have a lot of math are open. This notebook was the one we noticed with, but it might be just "long and lots of math" that's the problem.

Unfortunately it's tricky to pin down exactly what the problem is - I've tried opening it several times, locally and on the UC Berkeley, cloud-hosted campus hub, and I get inconsistent results. Sometimes my Lab instance gets sluggish while it's open, but other times it seems fairly normal.

Reproduce

To the extent it happens, I suggest

  1. Download that notebook
  2. Experiment with moving around, opening menus, having a few other notebooks open and switching back and forth, etc.

Expected behavior

UI should remain responsive and without high latency on interactive actions (menu open/hover, typing, tab switching, etc).

Context

I see folks in #4292 also discussing rendering issues, though there the conversation went more towards codemirror issues, so I'm not sure if the math aspect is the main problem, or "lots of codemirror instances", or some other combination of factors.

@ellisonbg mentioned we might want to test the KaTeX extension that I see @jasongrout has made recent commits to.

The notebooks the above is part of (all part of a class I'm co-teaching with @pbstark) have a ton of math so it's possible this alternate renderer is not enough for us, but it would be useful to know if it makes a significant difference in rendering speed and/or responsiveness of the UI. If anyone has a chance to test it out, that would be great. I'll report more if I do.

  • Operating System and version: macOS Big Sur.
  • Browser and version: Chrome 88 (current).
  • JupyterLab version: 3.0.5, 3.0.7.
@choldgraf
Copy link
Contributor

Also related to #7218 i think (speed improvements should be a major benefit of mathjax 3 http://docs.mathjax.org/en/latest/upgrading/whats-new-3.0.html#improved-speed)

@ellisonbg
Copy link
Contributor

ellisonbg commented Feb 7, 2021 via email

@fperez
Copy link
Contributor Author

fperez commented Feb 8, 2021

@ellisonbg sorry, not sure actually: I only noticed a few days ago and since it's with material from a new course, we've been on 3.0.x since the start. So I don't have a reference point from the 2.x series, never worked on these materials last semester when using Lab 2.x.

@telamonian
Copy link
Member

telamonian commented Feb 8, 2021

@fperez I tested your notebook out on 2.2.9, can confirm same problem with sluggishness, especially when clicking to open/close a menu.

I did a bit of profiling, and the problem is pretty obvious: there's waaaay too many DOM nodes. Your example notebook renders as ~19,000 separate nodes, around ~9,500 of which are created by mathjax alone; it seems that mathjax renders every separate symbol as a separate <span>.

As for why the menu performance in particular is bad, that's apparently due to the hitTest in @lumino/domutils that fires when you try to click on/outside of a menu/menubar:

  /**
   * Test whether a client position lies within an element.
   *
   * @param element - The DOM element of interest.
   *
   * @param clientX - The client X coordinate of interest.
   *
   * @param clientY - The client Y coordinate of interest.
   *
   * @returns Whether the point is within the given element.
   */
  export
  function hitTest(element: Element, clientX: number, clientY: number): boolean {
    let rect = element.getBoundingClientRect();
    return (
      clientX >= rect.left &&
      clientX < rect.right &&
      clientY >= rect.top &&
      clientY < rect.bottom
    );
  }

The call to element.getBoundingClientRect(); forces a reflow, which affects all ~19,000 aforementioned nodes. This ends up blocking the attempted menu interaction for a couple of seconds

@bollwyvl
Copy link
Contributor

bollwyvl commented Feb 8, 2021

Yeah, over on agoose77/jupyterlab-markup#14 we toyed with the idea of giving the option to delegate the actual math rendering out to a server-side cache... this would initially be far slower, and much more annoying to set up, but if cached properly, would result in many, many fewer DOM nodes, and you could use all of your domain-specific LaTeX stuff.

A half-step between this would be to use the MathJax SVG output, render off-screen (maybe in an iframe) and generate img tags. But the getBoundingClientRect is crushing.

@telamonian
Copy link
Member

A half-step between this would be to use the MathJax SVG output, render off-screen (maybe in an iframe) and generate img tags

I was also wondering if just sticking the mathjax svg output directly into a single inline <svg> element would might maybe improve performance. The reasoning would be that, at least for the purposes of the browser's layout engine, said <svg> element and all of its children would get treated as a single node.

I'm not 100% sure whether that's actually true or not, but I think it would be relatively simple to implement, and might be worth trying out?

@telamonian
Copy link
Member

telamonian commented Feb 8, 2021

But the getBoundingClientRect is crushing.

Separate from any mathjax question, it's probably also worthwhile for us to consider how we can improve hitTest:

  • Can hitTest be refactored to work without getBoundingClientRect?
  • If getBoundingClientRect is only needed in certain cases, we could perhaps add a hitTest(strict: bool = false) so that the reflow gets triggered only when truly necessary

@fperez
Copy link
Contributor Author

fperez commented Feb 8, 2021

Thx so much for that debugging work @telamonian, much appreciated!

I think what we're seeing is that as the tools get more robust (JLab, MathJax, JupyterBook), we're using them in ever more demanding cases, like these highly mathematical, essentially full-length interactive textbooks.

I'm curious as to whether the MathJax 3.x claimed rendering speed improvements (linked above by @choldgraf) would have any impact here. My issue wasn't so much initial rendering speed and more the resulting sluggishness of the whole UI afterwards, so I don't really know if their changes (welcome as they are) would have any positive effect here...

Have any of you tested MathJax 3.x already in JLab? Is it easy to try a swap, or have the APIs changed so much that even testing is a big lift?

@jasongrout
Copy link
Contributor

You can test Katex or MathJax 3 by installing an extension:

jupyter labextension install @jupyterlab/mathjax3-extension

or

jupyter labextension install @jupyterlab/katex-extension

@jasongrout
Copy link
Contributor

(I just published new versions of these extensions that update to MathJax 3.1 and Katex 0.12, respectively)

@dhirschfeld
Copy link
Member

You can test Katex or MathJax 3 by installing an extension

I'm very interested if the extensions improve the situation at all! 👀

@bollwyvl
Copy link
Contributor

if just sticking the mathjax svg output directly into a single inline

Yeah, the behavior/performance of inline SVG is sometimes a little funky, and can still get weird stuff happening, certainly between browsers. The big advantage, of course, is that CSS works.... but the biggest performance disadvantage is... CSS works. Moving almost all icons to inline SVGs has not been kind to performance... on the LSP completer, we offer Symbol icons (even have icon themes!) but pandas.<tab> (much less astropy.units.<tab>, even after it's cached, and made it all the way to the browser, is still extremely expensive.

Anyhow: re: generating MathJax out-of-band, perhaps better the iframe concept, it looks like it's possible to do at least the SVG output in a webworker. It's pretty short (partially because coffeescript... ahh, the memories) so a good example of prior art. Again, generating maximally one DOM node per $ or $$ block would help a lot. Also with a firmer boundary between render and display, caching would be possible.

yeah, even if we do get some of the mathjax stuff under control (which is important, given how easy it is to make them with markdown), there are a number of extensions that generate a lot of DOM. We (and other said extensions) also have a lot of CSS rules, some of which are pretty broad, yet match pretty specifically. I don't know of good tool for profiling CSS at the rule level, but it might be worth investigating, especially if it was simple for lab authors to use.

how we can improve hitTest

avoiding GBCR is indeed clutch. It looks like it's used in <10 places in lumino (the datagrid one is separate) and even fewer in lab core (although in one helper, it's inside a loop!).

if i recall correctly, this was ironically done for performance reasons to avoid making lots and lots of event listeners in lumino vdom, as they do stack up, for sure, but they do in any virtual DOM. It certainly seems reasonable to have 1000s of file listings, though 1000s of menu items seems far less likely. I know the d3 folks figured out a bunch of this through other means, so might have some prior art. I also don't know about whether the quickly-googled alternatives have similar problems, but generally it's possible that some form of caching would work, though the various virtual DOM integrations would get complicated, and might make caching (the other approach suggested) very hard to reason about.

@krassowski
Copy link
Member

Yes, hitTest can work without getBoundingClientRect (using intersectionObserver; it requires a modern browser; we already introduced intersectionObserver elsewhere, with a fallback; we could have a fallback in lumino too).

But why do we need to call hitTest in the first place? The following lines:

  private _evtMouseMove(event: MouseEvent): void {
    // Hit test the item nodes for the item under the mouse.
    let index = ArrayExt.findFirstIndex(this.contentNode.children, node => {
      return ElementExt.hitTest(node, event.clientX, event.clientY);
    });
    //...
  }

end up re-implementing the "mouse over" and "mouse out" events for children; there are some small benefits of this approach (children not telling parents what to do + avoiding a few event listeners more) but not worth it at such a performance hit. Fewer listeners are nice but not at a cost of firing another listener thousands times (at every single pixel the mouse moves over in the menu...).

There are a few solutions we could try:

  1. cache the getBoundingClientRect calls - IMO not worth it, as discussed above it will cause headaches if user scrolls page etc.
  2. add a debouncer to avoid reflow at every single pixel (getting a tradeoff between lower snappiness when no notebooks are open and relatively higher snappiness when heavy notebooks are open... though we could count how many nodes are on there and use some log10(#nodes) as the debouncer delay avoiding the dilemma)
  3. use IntersectionObserver to either:
    • a) re-implement getBoundingClientRect but without reflow
    • b) actually test for intersection between a dummy 1px x 1px transparent node placed on cursor position on mouse move (if we want to continue re-implementing the mouseover/mouseout)
  4. alternatively, we could just leave the mouseover/mouseout implementation to the browsers - they are really good at it.

The solutions (1) and (2) would be backward-compatible and can backported to JupyterLab 3.x; I am not sure if (3a) would need to break interfaces (we need to tell the observer to observe but we could just spawn observers ad hoc - but this might come with a penalty and defeat the purpose). Solutions (3b) and (4) will require breaking changes and we would need to bump lumino major and wait for JupyterLab 4.0.

@fcollonval should we discuss this one at the next performance meeting? Do we have a test case for this one?

avoiding GBCR

Grace Bible Church Rugby? Gross Building Coverage Ratio?

PS. this issue is also annoying for notebooks with no math at all, just a lot of content. I think that some browsers might be more severely affected than others.

@krassowski krassowski added the bug label Dec 17, 2021
@jasongrout
Copy link
Contributor

Instead of a hit test, could we do a node.contains test on the mouse move target?

@krassowski
Copy link
Member

Yes, that's a great suggestion. I tried that in jupyterlab/lumino#273 but it turns out that it does not solve the issue for menu(bar) responsiveness because other operations trigger the reflow even if we eliminate ElementExt.hitTest (described in the draft PR).

My assumption that using getBoundingClientRect in a loop is a culprit was also wrong - it looks like only happens if needed so
consecutive calls should not matter (if the DOM does not evolve).

Maybe the next step would be to try to understand which styles are expensive and cause the reflow to take so long? This introduction to performance mentions web components and BEM as possible approaches to tackle the issue but does not explain how to narrow down problematic rules. Might be worth a dive to see what tools could help us understand why layout shift takes so long (other than because there are many nodes).

@krassowski
Copy link
Member

krassowski commented Dec 22, 2021

Analysis:

I am currently focusing on the delay visible when moving mouse out of the menu and the over again (this is universal problem when the test notebook is open and affects all menus, including menu bar and context menu):

slow-baseline

After lower than expected improvements in jupyterlab/lumino#273 and failure to eliminate this even when commenting out handlers for mousemove, mouseenter and mouseleave I went a step further and removed event listeners from lumino's Menu for mousemove, mouseenter and mouseleave altogether. To my surprise the issue did not go away - it was still present when looking in the profiler; we don't see any layout shifts but a lot of long Recalculate Style tasks; those are preceded by Schedule Style Recalculation which helps to narrow down where it is coming from.

Screenshot from 2021-12-22 22-20-39

The profiler does not show why those come to be - just that they follow browser-native HitTest (and precede the actual event emission). The raw JSON profile file hints that the HitTest goes to the notebook cell (not unexpectedly, this is where my cursor lands after leaving the menu) and also mentions intersection observer (IntersectionObserverController::computeIntersections) kicking in from time to time (due to lazy notebook rendering I suppose):

The following experiment suggests that this might be triggered by HitTest when mouse leaves the menu (and then HitTest for some weird reason causes style recalculation on Chromium/Blink):

  • on top of initial changes in lumino#273 add a <div class="test"> with z-index=z-index of menu - 1
  • make that div position:absolute and cover entire viewport (height:100%; widht: 100%) so that it occludes the rest of the DOM (except for the menu)
  • optionally add background: grey; opacity: 0.5 just to see the div
  • see that moving mouse out of the menu and back again does not cause the delay anymore

now-smooth

And this time there is no style recalculation anymore nor scheduling of it:

Screenshot from 2021-12-22 22-18-45

Now the behaviour of Hit test is standardized by W3C: https://www.w3.org/wiki/Hit_Testing, but it is not well understood topic when it comes to performance optimization. There is one question on SO which concerns the Hit Test taking a lot of time itself, but in our case the problem is not the presence of Hit Test but that it sometimes schedules style recalculation.

So why a Hit Test might schedule a style recalculation even though nothing changed in the DOM? Why does it not happen every time but just some times? Well :hover pseudo selector might be the answer. We can test that by adding:

<style>.test:hover{background:red!important}</style>

This indeed restores the Schedule Style Recalculation and Recalculate Style tasks (although this time style recalculation is super fast because only our test div is affected):

hover

Screenshot from 2021-12-22 22-17-06

While I only tested on Chromium-based browsers, I would not be surprised if other engines are affected too.

@krassowski
Copy link
Member

krassowski commented Dec 22, 2021

Proposed solution

Could we add a <div> occluding background DOM when menu is open? The div would be effectively transparent (this could be implemented e.g. with opacity: 0.01) and clicking on it would detach it from DOM and close the menu. This means that when user has a menu open and clicks outside, e.g. on a link, it would not open (unless they click again after menu gets closed).

The UX would be consistent with how native menus (both context menus and application-level menus) work in Chrome (on Ubuntu), in GNOME and I suspect in many other applications/environments: neither hover nor click works on background elements when menu is open. Firefox does a bit of both: clicks do not work on background elements when context menu is open, but hover styles do; clicks do work in Firefox though when the topbar menu is open (as do hover styles).

And it be super fast regardless if the number of nodes in the DOM is 6000 or 600000.

Edit: just in case if someone was wondering, setting pointer-events: none; on the jp-LabShell does not solve the issue.

@fcollonval
Copy link
Member

Thanks for digging up on this one and sharing your experiment result. I'm 👍 for your proposed solution.

@mlucool
Copy link
Contributor

mlucool commented Feb 25, 2022

Thanks for sharing this post in jupyterlab/benchmarks#90 and your great analysis. I also think we should go ahead with @krassowski's proposed solution. While not clearly ideal, if the code is well documented as to why we added this DIV we can always revert it if we find a better solution. @krassowski you volunteering to make this change or did you need some help?

FWIW for chrome on windows, when you have a menu open (e.g. context menu) the menu closes on keypressdown so you can effectively directly click on links that seems to listen to keypressup.

@rtrg
Copy link

rtrg commented Mar 11, 2022

Alternatively:

Using content-visibility:auto css on .jp-Cell helps here by avoiding calculating style/layout for cells' subtree DOM which are not in viewport and shouldn't have any sideeffects. This makes sure that only visible cells go through style/layout recalculations on mouse hover and out of menu bar which doesn't have any perceivable delay.

The content-visibility property controls whether or not an element renders its contents at all, along with forcing a strong set of containments, allowing user agents to potentially omit large swathes of layout and rendering work until it becomes needed. It has the following values:

Specification is WIP but majority of the browsers implemented it except firefox and seems like firefox doesn't have hit test style/layout recalculation issue.

image

Having content-visibility:auto css should also help in other common user interactions which trigger css reflow (apart from menu bar delay issue)

Any cell outside of view port is considered to have 0 height because of which scroll jitters, to avoid scroll jitter we need to combine above with contain-intrinsic-height (not supported in firefox and safari)

We can use place holder cells height (let's say 70px) to start with for every cell which is not in viewport and hasn't rendered yet even once.

Since cells can be of varying length this constant 70px height assumption won't work well, to overcome this contain-instrinsic-height: auto 70px can be used which gets rid of 70px height assumption as soon as cell gets into viewport (for ex: because of scroll), computes its actual height and uses this newly computed height as new reference instead of 70px when it goes out of viewport.

%%javascript
function addCss(cssCode) {
    const style = document.createElement('style');
    style.innerHTML = cssCode;
    document.head.appendChild(style);
}
 
// This uses experimental CSS that is not supported by all browsers.
// By setting content-visibility: auto, when the cell is off-screen,
// style checks are avoided. We set an intrinsic height so that
// scrolling is not too jittery.
addCss(`
    .jp-Cell {
        content-visibility: auto !important;
        contain-intrinsic-height: auto 70px;
    }
`);
`)

@cantagallo
Copy link

cantagallo commented Jun 28, 2022

Hi all,
On a JHub installation we are observing for several users the very same issue (very long "Recalculate style") also on JLab 3.4.
Any update on a possible fix? The solution proposed by @krassowski looks as a good candidate to me
thanks!

@xgdgsc
Copy link

xgdgsc commented Sep 7, 2022

It can be even slower when open a "New view for notebook".

@krassowski
Copy link
Member

Coming back to this issue after some more exploration on the Lumino side and checking with latest JupyterLab 4.0.0-alpha28:

  • The lag is still noticeable with the original test notebook when using the version from the time of reporting;
    • to reproduce you need to wait for all math expressions to render
    • for easier reproduction, disable the virtual windowed notebook (Settings → Notebook → Windowing mode → none). Windowed notebook reduces the style computation cost by reducing the number of nodes: there are ~52k nodes with the tests.ipynb notebook open (fully rendered), regardless of windowing status; with windowing there are 5.7k attached nodes, without there are 32.5k attached nodes (as measured with document.getElementsByTagName("*").length); simplifying a bit windowing allows us to open ~6 notebooks before arriving at the same problem (iframe issues aside).
  • Issues with long style recalculation mostly affect Chromium browsers.
  • It is impossible to completely avoid layout revalidation and the need for style recalculation for menus without extensive modifications (moving the menu rendering completely to the composer thread; that restricts us to exclusively modify transform and opacity properties of the menu and forces use of will-change
  • Implementing the hover blocking workaround proposed in Sluggish UI when long, math-heavy notebooks are open. #9757 (comment) is not trivial without breaking the current UX (the cost is that we would need to break the menubar behaviour or spend many more hours here to get the desired effect).

Further analysis using chrome://tracing/ narrows the problem with styles performance within Document::recalcStyle to a large number of stylesChanged and rulesMatched - higher than I would expect:

{baseStylesUsed: 0,
 customPropertiesApplied: 0,
 elementsStyled: 19859,
 independentInheritedStylesPropagated: 0,
 matchedPropertyApply: 0,
 matchedPropertyCacheAdded: 0,
 matchedPropertyCacheHit: 5157,
 matchedPropertyCacheInheritedHit: 811,
 pseudoElementsStyled: 14,
 rulesFastRejected: 1023725,
 rulesMatched: 40466,
 rulesRejected: 644729,
 stylesAnimated: 0,
 stylesChanged: 1049,
 stylesUnchanged: 18818}

Searching the Chrome bug tracker I found:

Concatenating style tags: does not help

Without any extensions (in dev mode) JupyterLab 4.0 has 144 style tags (document.querySelectorAll('style').length). Concatenating the tags on runtime does not solve the problem; to test it locally use:

const individualStyles = [...document.querySelectorAll('style')];
const concatenated = individualStyles.map((x) => x.innerHTML).join('\n')
var style = document.createElement('style');
style.type = 'text/css';
style.innerHTML = concatenated;
document.getElementsByTagName('head')[0].appendChild(style);
individualStyles.map(x => x.remove());

There may be some advantage of doing so for other performance issues, but the benefits were not seen for this specific issue.

Disabling styles one by one: bingo

Removal of just three style tags reduces the style re-computation time by over an order of magnitude; in 4.0.0-alpha28 these can be removed with:

let styles = [...document.querySelectorAll('style')];
styles.slice(104, 105).map(x => x.remove());
styles.slice(119, 121).map(x => x.remove());

the ordering of style tags will vary between versions (and possibly extensions installed/plugins disabled).

These are two style sheets from MathJax and styles for .jp-RunningSessions sidebar.

We already know that the trigger is :hover pseudostate. Removing all styles with :hover pseudoselector from the three stylesheets solves the problem. The minimal subset of rules to remove to regain responsiveness is:

.MathJax_Hover_Arrow:hover span {background-color: #CCC!important}

and

.MathJax_MenuClose:hover span {background-color: #CCC!important}

and

.jp-RunningSessions-shutdownAll.jp-mod-styled:hover > span {
  background-color: var(--jp-layout-color2);
}

.jp-RunningSessions-shutdownAll.jp-mod-styled.jp-mod-disabled:hover > span {
  background: none;
}

Rules which include :hover but do not include span do not contribute to the problem:

.MathJax_MenuClose:hover {color: white!important; border: 2px solid #CCC!important}
.MathJax_MenuClose:hover:focus {outline: none}

.MathJax_Hover_Arrow:hover {color: white!important; border: 2px solid #CCC!important}

.jp-RunningSessions-item:hover {
  background-color: var(--jp-layout-color2);
}
.jp-RunningSessions-item:not(:hover) .jp-RunningSessions-itemShutdown {
  visibility: hidden;
}
.jp-RunningSessions-sectionList
  .jp-RunningSessions-item
  .jp-Button.jp-RunningSessions-itemShutdown:hover {
  background: var(--jp-layout-color3);
}

Note that it does not matter if .jp-RunningSessions-shutdownAll is attached to the DOM or even exist - the problem is in existence of the selectors in the stylesheet. This might be a bug in Chrome's blink-style component, somewhat similar to https://bugs.chromium.org/p/chromium/issues/detail?id=542082 where the sole existence of a style for which there was no element was causing mayhem.

The .jp-RunningSessions-shutdownAll.jp-mod-styled:hover > span was just introduced in #13074 (CC @fcollonval) so on 3.x branch it's only the MathJax styles which are problematic.

With this new knowledge I was able to create a minimal reproducer which works regardless of notebook windowing, which is a notebook with a single cell containing the following code and its output:

from IPython.display import HTML
display(HTML(
    ''.join([
        '<span>x</span>'
        for _ in range(50000)
    ])
))

Recalculating style on hover takes ~350 ms for me with all the styles and only ~8ms with the styles mentioned above removed. This highlights that the default MathJax styles compromise performance (due to a potential Chromium/blink performance issue) even if no MathJax expressions are on the page.

Note: creating a large SVG with 50k nodes in itself does NOT recreate the problem:

from IPython.display import HTML
from random import random
display(HTML(
    '<svg>' +
    ''.join([
        f'<circle cx="{random() * 100}" cy="{random() * 100}" r="{random() * 10}" stroke="black" stroke-width="1" fill="red" />'
        for _ in range(50000)
    ]) +
    '</svg>'
))

Taken together, this looks similar to a bug in Chromium: 1248496: Pesudo hover on parent element affects all the child elements if the child contains the form element, but from a different code path.

Indeed in the above <span> reproducer the Recalculate Style has Elements Affected 50459 and then if we remove the problematic styles it only has Elements Affected 259.

I was able to modify the reproducer from Chrome issue 1248496 to demonstrate the problem we are hitting:

<html>

<head>
  
  <style>
    .editIcon {
      opacity: 0;
    }

    .container:hover .editIcon {
      opacity: 1;
    }

    .unused-class:hover > span {
      background: red;
    }
  </style>
</head>

<body>

  <div class="container" id="test">
    <div>
      <div class="editIcon">##</div>
    </div>
  </div>

</body>
<script>
  let container = document.getElementById('test')
  for (let i = 0; i< 100000; i++) {
    let newDiv = document.createElement('div')
    newDiv.appendChild(document.createTextNode(`sampleDiv ${i+1}`));
    let newInput = document.createElement('span');
    newInput.innerHTML = 'x';
    newDiv.appendChild(newInput);
    container.appendChild(newDiv);
  }
</script>
</html>

After recording performance profile when hovering over ## and then leaving it, the Elements Affected for Recalculate Style should be 2 but is 1000002. Removing .unused-class:hover span solves the problem. It happens regardless of using .unused-class:hover > span vs .unused-class:hover span.

@krassowski
Copy link
Member

There are also rules causing sluggish UI in presence of many <svg> or many <div> nodes:

<svg>

Single rule present since JupyterLab 2.0 (icons refactor):

.jp-icon-hoverShow:not(:hover) svg {
display: none !important;
}

reproducible with:

from IPython.display import HTML
display(HTML(
    ''.join([
        '<svg></svg>'
        for _ in range(50000)
    ])
))

<div>

Rules introduced in JupyterLab 3.1 (RTC) and 3.3 (Settings UI):

.jp-CodeMirrorEditor .remote-caret:hover > div {
opacity: 1;
transition-delay: 0s;
}

'&:hover div': {
fontWeight: 600,
color: 'var(--jp-ui-font-color0)'
},

reproducible with:

from IPython.display import HTML
display(HTML(
    ''.join([
        '<div></div>'
        for _ in range(50000)
    ])
))

I did not find other HTML tags to be affected, but since a large notebook with visualisation may have thousands of div, svg and span elements (where spans are inflated by MathJax v2) all of this likely compounds to create a sluggish UX. This seems to align with results from invalidation trace which showed these three tags.

Screenshot from 2022-10-01 17-25-24

Again, windowing helps here but does not solve the problem if there are many nodes in a single output (or multiple outputs visible at once). Finally, extensions may introduce additional offending styles.

Note: if there is a qualified selector in the HTML tag after :hover, like in: .jp-toc-numberingButton:hover button.lm-mod-toggled.jp-Button.jp-mod-minimal then there is no problem.

@fperez
Copy link
Contributor Author

fperez commented Oct 10, 2022

As I indicated in #13159, huge, huge ❤️ to @krassowski for this deep-dive into the problem and providing a fix. Many will benefit from these improvements, looking forward to testing it soon!

@krassowski
Copy link
Member

krassowski commented Oct 10, 2022

Thanks! Just to clarify this issue was not fully resolved by #13159 since:

So while we did make a lot of progress, the issue got closed because we track the remaining tasks elsewhere, not because the problem is fully solved.

One final thing we could do to prevent specifically math-heavy notebooks from degrading performance is to use MathJax 3 in JupyterLab 4.0 - does anyone here have any comments (i.e. tried out MathJax 3 as a replacement as suggested earlier and encountered any issues or not)?

@jasongrout
Copy link
Contributor

FYI, MathJax is now doing prereleases of mathjax version 4.0. Mathjax 4.0 implements the linebreaking that we identified as a possible compatibility issue at #7218. See mathjax/MathJax#2312 (comment) for more discussion about the linebreaking and the release notes for mathjax 4.0a1. As for performance issues, MathJax 4.0 release notes say that it does some font load asynchronously now, which may speed up the initial loading.

@krassowski
Copy link
Member

Comparing MathJax2 vs MathJax3 with the test.ipynb (math-heavy) notebook open, using jupyterlab-ui-profiler: the performance improvements on migration to MathJax 3 are notable in JupyterLab 3.5 with Chrome:

Scenario Speedup MathJax 2 MathJax 3
DOM elements 36% 34023 21735
sidebar open/close* 59% 432 [415.9 – 476.4] 172.9 [141.3 – 205.7]
menu open 39% 29.3 [26.3 – 36] 18 [16.9 – 20.8]
tab switch 22% 87.9 [78.5 – 98.6] 68.9 [64.8 – 73.3]
completer** 16% 106.9 [100.2 – 118.2] 89.9 [85.6 – 96.4]

Times given in milliseconds IQM [Q1 – Q3]

*) proxy for resizing main area widgets
**) 100 tokens, each 5 characters long

This effect appears entirely attributable to reduction in number of DOM nodes; when running benchmarks with 50k div, span and svg nodes and no math expressions I only see small, likely spurious, differences:

Scenario Speedup MathJax 2 MathJax 3
DOM elements 0% 156507 156457
sidebar open/close* 0% 589.2 [554.1 – 637] 585.5 [563.1 – 614]
menu open -5% 42 [36.5 – 54.2] 44.1 [40.4 – 51.2]
tab switch 7% 124.4 [119.1 – 132.6] 115.1 [105.4 – 122.9]
completer** 0% 125.9 [117.1 – 148.6] 125.8 [113 – 142.8]

As for the alternative, Katex, it does not seem as promising as MathJax 3, but is still faster than MathJax 2:

Scenario Speedup MathJax 2 Katex
DOM elements 14% 34023 29294
sidebar open/close* 38% 432 [415.9 – 476.4] 268.5 [233 – 366.7]
completer** 10% 106.9 [100.2 – 118.2] 95.5 [87.1 – 99.2]
menu open 7% 29.3 [26.3 – 36] 27.1 [23.8 – 31.6]
tab switch 6% 87.9 [78.5 – 98.6] 82.3 [75.8 – 92.7]

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 29, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.