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

noContent > * selector slows down initialization #655

Closed
heikomat opened this issue Jun 24, 2024 · 9 comments
Closed

noContent > * selector slows down initialization #655

heikomat opened this issue Jun 24, 2024 · 9 comments

Comments

@heikomat
Copy link

heikomat commented Jun 24, 2024

Describe the bug
In my react application, i have some OverlayScrollbars.
It seems like this selector (only the one with > *) about doubles the time it takes to initialize the scrollbars:

[data-overlayscrollbars-viewport~='noContent']:not(#osFakeId) > * {

I noticed that this selector was checked a lot, but never actually matched (in my case) when trying out chromes new enable css selector stats-option

To Reproduce

  1. Have a react-application with a scroll-container that already contains children (more children = better)
  2. have the overlay-scrollbars initialize defered (optional, but makes the issue better measurable)
  3. measure how long the initialization takes
  4. remove the mentioned selector and retest the initialization
with the selector without the selector
table-before Screenshot 2024-06-24 at 14 38 27

Expected behavior
I would like the scrollbar initialization to be as fast as possible

Examples
I'm currently at work and don't have time to create a working sandbox. Sorry :(
I hope the info i collected is sufficient

Environment

  • Used Operating System(s): MacOS 14.5 on an M2 Max MacBook Pro
  • Used Browser(s) (with version): chromium 125
@heikomat
Copy link
Author

heikomat commented Jun 24, 2024

Oh, and i just noticed, when switching back to version 2.7.2, which we currently use in production, i cannot find these blocking "recalculate style-parts during initialization" in my profilings at all.

Update: Found them, but they are in another place in the profiling. They also seem to be way faster. They seem to all affect 0 elements, while the blocking parts in the original post affect ~1300 elements (in my tests, probably depends on the number of children in the ScrollContainer)

Screenshot 2024-06-24 at 15 08 40

@KingSora
Copy link
Owner

KingSora commented Jun 25, 2024

good day @heikomat :)

I see.. short explanation why this selector is needed and what it does:

  • Sometimes users initialize OverlayScrollbars to an element which has a non-default scroll direction. Examples for this are direction: rtl, flex-direction: column-reverse, flex-direction: row-reverse and any writing-mode values which are not horizontal-tb. Those can also be combined, for example an element with the style direction: rtl; flex-direction:row-reverse will end up with a default scroll direction because those two "cancel out" so to say.
  • In such cases I need to detect the scroll coordinates, whats the scroll position when the content was not scrolled at all and whats the position when it was scrolled to the end. Browsers right now align in those coordinates but this wasn't the case in the past and (as far as I can see) is also not standardized. If the detection of those coordinates fails I always fall back to coordinates which represent a default scroll direction.
  • Currently I'm using exactly this selector to be able to detect those scroll coordinates.

As you probably already figured out, this doesn't really affect the functionality of your app, because you're probably only using the default scroll direction. Because the coordinates will always fallback to the default scroll direction your app will also continue to work even without the style. So for you this whole detection brings 0 benefit and only makes the update cycle slower.

This detection is already cached and optimized in how many times it runs, but is must always run at least once (thats why you mostly will only feel the performance impatc on the initialization) or when updating with update(true)

I'm having a few ideas of how to improve this:

  • Provide a new option which hints the plugin your intended scroll direction and thus the detect doesn't need to run
  • Somehow detect whether the element even can have a non-default scroll direction by checking the styles mentioned above before detecting (could be potentially less future proof as we could get new styles which influence scroll-direction)
  • Find a better detection of the scroll-coordinates (I tried many things in this regard, so no idea if thats even possible)

Do you have any thoughts or preferences of how to solve this problem?

@heikomat
Copy link
Author

An option to hint the scroll-direction would definitely work for me and sounds like a simple enough solution.

Only thing i find strange is, that the problem doesn't seem to exist in 2.7? or maybe i'm just not seeing it in the profiling.

@KingSora
Copy link
Owner

KingSora commented Jun 26, 2024

@heikomat that make sense since this feature with the non-default scroll-direction was added in v2.8.0..

I still need to think about this a little bit, since a separate option could be hard to explain / discover for not experienced users.. this problem should be fixed in the next release though

@heikomat
Copy link
Author

I understand. Take your time and thanks a lot! :)

@KingSora
Copy link
Owner

KingSora commented Jul 2, 2024

@heikomat I've just released v2.9.2 which includes a fix for this issue. You don't have to do anything in order of this optimization to work. I'm now checking whether its likely that the scroll element has a non-default scroll direction and if not the measuring code is not executed.

Please try it out and give feedback :)

@heikomat
Copy link
Author

heikomat commented Jul 4, 2024

@KingSora thanks!
It might take a couple days before i can test that. Expect a response within the next week :)

@heikomat
Copy link
Author

heikomat commented Jul 11, 2024

Whatever you did seems to have helped :)
version 2.9.2 is now slightly faster than 2.7.2, while 2.9.1 was measurably slower.

I conducted all the following tests under the following conditions:

  • MacBook Pro M2 Max
  • Chromium 125
  • 4x slowdown in dev-tools
  • production-vite-build of my application

In my tests, i navigate within a single-page-application to a page that renders a container with defered OverlayScrollbars and elements inside. There seems to be two scrollbars initializing, but that doesn't matter to much, as the results remain comparable.
The measured time is from the first "Task" that contains OverlayScrollbars-logic until all work is done (as can be seen in the screenshots)

I did two runs for the versions 2.7.2, 2.9.1 and 2.9.2. Here are the results:

Version 2.7.2
os-2 7 2_1 os-2 7 2_2
Version 2.9.1
os-2 9 1_1 os-2 9 1_2
Version 2.9.2
os-2 9 2_1 os-2 9 2_2

As you can see, in my specific test-setup, version 2.9.2 uses measurably less time for scripting compared to 2.7.2 while requiring about the same amount of rendering.
2.9.1 is the worst of the three, requiring as much scripting as 2.7.2 and nearly double the rendering time.

In my tests, initializing an OverlayScrollbars still takes some time (~70ms) but that might very well be an issue with the elements i'm rendering in there and not with OverlayScrollbars. Just for reference, Here is a zoom-in of the last "Task" of the last 2.9.2-measurement:
Screenshot 2024-07-11 at 08 47 32

These tests and writeups take time, but if it helps you with this Project it's well worth it.
Thank you for fixing this, and making it the fastest version i've tested :)

Closing this issue as i consider it solved

@KingSora
Copy link
Owner

@heikomat I really appreciate the work you put into the tests and comparisons between versions. It really helps me to improve the quality / performance of the plugin.

I'm glad the new version solved this specific issue.

Please feel free to open as many bug / improvement issues as you want, I'll try to follow up on every single one :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants