KingSora / OverlayScrollbars

A javascript scrollbar plugin that hides the native scrollbars, provides custom styleable overlay scrollbars, and preserves the native functionality and feel.
https://kingsora.github.io/OverlayScrollbars
MIT License
3.71k stars 215 forks source link

noContent > * selector slows down initialization #655

Closed heikomat closed 1 week ago

heikomat commented 3 weeks ago

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: https://github.com/KingSora/OverlayScrollbars/blob/d33ea868ce9800f051f00a8015df67e4b3ae3f8e/packages/overlayscrollbars/src/styles/structure.scss#L145

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

heikomat commented 3 weeks ago

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 commented 3 weeks ago

good day @heikomat :)

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

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:

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

heikomat commented 3 weeks ago

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 commented 3 weeks ago

@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 commented 3 weeks ago

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

KingSora commented 2 weeks ago

@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 commented 2 weeks ago

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

heikomat commented 1 week ago

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:

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](https://github.com/KingSora/OverlayScrollbars/assets/20770029/aee60581-9785-4ca4-beef-c1fc4f244543) | ![os-2 7 2_2](https://github.com/KingSora/OverlayScrollbars/assets/20770029/6ff67aeb-e814-4cb0-9a20-579938af02a9) | | --- | --- |
Version 2.9.1 | ![os-2 9 1_1](https://github.com/KingSora/OverlayScrollbars/assets/20770029/dc6e4378-2e9d-47cd-9c3b-d0da37c8ceb9) | ![os-2 9 1_2](https://github.com/KingSora/OverlayScrollbars/assets/20770029/4fac3d92-dcd8-4e29-ba53-314ac35a2c83) | | --- | --- |
Version 2.9.2 | ![os-2 9 2_1](https://github.com/KingSora/OverlayScrollbars/assets/20770029/ec8b1f57-459e-4c82-9d6b-0e9acbaabf80) | ![os-2 9 2_2](https://github.com/KingSora/OverlayScrollbars/assets/20770029/59f21601-9cf6-4863-963d-9a5cabd4d10d) | | --- | --- |

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 commented 1 week ago

@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 :)