lumino icon indicating copy to clipboard operation
lumino copied to clipboard

Handle deferred processing when `document` is hidden

Open afshin opened this issue 2 years ago • 4 comments

Context

We merged a recent PR from @thetorpedodog (thank you!) modifying the Poll class to stop using requestAnimationFrame/setImmediate in favor of setTimeout every time:

  • https://github.com/jupyterlab/lumino/pull/395 in order to resolve:
  • https://github.com/jupyterlab/lumino/issues/392

Problem

Upon further reflection and talking through this with @blink1073 and @fcollonval, I think this may not solve the original issue that Paul is trying to solve. Because:

  • Poll.IMMEDIATE was a sentinel value (0) that told the Poll to use requestAnimationFrame/setImmediate
  • However that was only happening when polling was explicitly so to happen immediately. This happens nowhere in JupyterLab core as far as I can see.

Most polls are set to reasonably long intervals. The more likely reason is two-fold:

  • Use of the when-hidden standby mode for Poll instances takes place immediately when the window is hidden.
  • Even if crucial Poll instance tick one more time, all Widget rendering in Lumino applications happens using the MessageLoop, which uses requestAnimationFrame/setImmediate.

Proposal

  • Let's release JupyterLab 4.0.0a29 with @thetorpedodog's fix to see if it resolves the issue. My suspicion is that it will not.
  • Let's release JupyterLab 4.0.0a30 with the following changes:
    • Create a schedule() and unschedule() abstraction utility to use in Poll and MessageLoop that picks the defer function based on whether the client is a browser and whether it is hidden.
    • Modifying the behavior of the when-hidden standby to tick a few more times after being hidden (perhaps a default value of 1 lingering tick that can be changed by a client)

This way there are two easy to validate versions of JupyterLab that attempt to resolve this issue that we can compare.

afshin avatar Sep 09 '22 14:09 afshin

Note: The PR was backported to the 1.x branch. Will that be an issue?

vidartf avatar Sep 09 '22 17:09 vidartf

Modifying the MessageLoop to use setTimeout when the window is hidden but to use requestAnimationFrame/setImmediate otherwise

Isn’t this essentially equivalent to just using setTimeout(..., 0) all the time?

Also: leaving myself a note to trace where the requestAnimationFrame stuff came from, because I am almost certain it went through the Poll path, though it might have gone through the message loop and then a Poll.

thetorpedodog avatar Sep 09 '22 17:09 thetorpedodog

@vidartf,

Note: The PR was backported to the 1.x branch. Will that be an issue?

We will not release a patch release of the 1.x branch until we have an answer to this.

We'll just do 2.0 alphas along with JupyterLab 4.0 alphas in lock-step to make sure users don't get hit with unintended side-effects.


@thetorpedodog,

Modifying the MessageLoop to use setTimeout when the window is hidden but to use requestAnimationFrame/setImmediate otherwise

Isn’t this essentially equivalent to just using setTimeout(..., 0) all the time?

It isn't equivalent to always using setTimeout because presumably the majority of the time that the MessageLoop is dispatching messages is when the application is visible, not when it is hidden. Both in terms of performance and reliability, we should be using requestAnimationFrame/setImmediate except when the document is hidden and incurs unintended consequences.

afshin avatar Sep 09 '22 18:09 afshin

Also: leaving myself a note to trace where the requestAnimationFrame stuff came from, because I am almost certain it went through the Poll path, though it might have gone through the message loop and then a Poll.

Your theory sounds plausible. But if you do discover it's some path that's not obvious right now, please report back!

afshin avatar Sep 09 '22 18:09 afshin