-
-
Notifications
You must be signed in to change notification settings - Fork 128
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
Avoid menu layout trashing by moving DOM queries #432
Avoid menu layout trashing by moving DOM queries #432
Conversation
2587990
to
3fd6930
Compare
3fd6930
to
4958065
Compare
The DOM queries will execute before subsequent DOM modifications to avoid layout trashing when opening or switching menus.
4958065
to
58f3374
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot @krassowski
The impact is really impressive on Chrome.
I left a couple of questions.
7d427b8
to
a50616a
Compare
a50616a
to
1dba9cd
Compare
Green now. The test failure was actually just my mistake when porting the tests not an actual issue in the code 🤦 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks a lot @krassowski
Thank you @fcollonval. Do you think that we can backport this on 1.x and include in JupyterLab 3.5? It includes a new public API member, so I am not sure here. |
Sure thing Extending or adding API is allowed as they do not break backward compatibility - especially in this case as they are not really meant to be used outside of Lumino. |
@meeseeksdev please backport to 1.x |
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon! Remember to remove the If these instructions are inaccurate, feel free to suggest an improvement. |
…queries * Move/cache DOM queries to prevent menu layout trashing The DOM queries will execute before subsequent DOM modifications to avoid layout trashing when opening or switching menus. * Use simpler transient cache, move update requests (cherry picked from commit 55e87ca)
…queries * Move/cache DOM queries to prevent menu layout trashing The DOM queries will execute before subsequent DOM modifications to avoid layout trashing when opening or switching menus. * Use simpler transient cache, move update requests (cherry picked from commit 55e87ca)
) * Move/cache DOM queries to prevent menu layout trashing The DOM queries will execute before subsequent DOM modifications to avoid layout trashing when opening or switching menus. * Use simpler transient cache, move update requests (cherry picked from commit 55e87ca)
This improves menu opening and switching performance as proposed in #329 (comment).
How to test
@lumino/virtualdom
and@lumino/widget
against JupyterLab HEADgh-9757-reproducer.ipynb
, optionally changen=100_000
(this is what I am using)Recalculate Style
event durationSwitching Menu (menubar)
Chrome (x6 CPU slowdown)
Firefox (no throttling)
Opening Context Menu
Chrome (x6 CPU slowdown)