ganlanyuan / tiny-slider

Vanilla javascript slider for all purposes.
MIT License
5.25k stars 785 forks source link

performance: forced-reflow while tns init #176

Open staabm opened 6 years ago

staabm commented 6 years ago

Issue description:
when initializing the slider one can see some "forced-reflow errors" in the chrome dev-tools which indicate some clientside performance problems with tiny-slider.

Demo link/slider setting:
please find my exported chrome developer tool profile tiny-slider_chrome-profile.zip

grafik

grafik

see https://developers.google.com/web/fundamentals/performance/rendering/avoid-large-complex-layouts-and-layout-thrashing#avoid-forced-synchronous-layouts for further background.

Tiny-slider version: 2.6.0 Browser name && version: Chrome 67.0.3387.0 OS name && version: Windows10 x64

staabm commented 6 years ago

our init js code looks like

    function initSlider5356() {
        var sliderRoot = document.querySelector('#cmsmoduleid5356');

        var sliderContainer = sliderRoot.querySelector('.slider');

        if(sliderContainer)
        {
            var slider = tns({
                container: sliderContainer,
                controlsText: ['', ''],
                speed: 1000
            });

            sliderContainer.classList.remove('initialstate');
        }

        var carSearchSlider = sliderRoot.querySelector('.car_search_slider');

        if(carSearchSlider) {
            var controls = carSearchSlider.nextElementSibling;
            var slider = tns({
                container: carSearchSlider,
                controlsContainer: controls,
                controlsText: ['', ''],
                loop: false
            });
        }
    }
staabm commented 6 years ago

2 takeaways:

ganlanyuan commented 6 years ago

Hey, thanks for the report.

Thanks

staabm commented 6 years ago

I can confirm a improvement regarding reflows... they are less expensive now.. ? getViewportWidth() is still showing up in the chrome dev panel, though:

grafik

chromes measures the following perf hit: el.clientWidth is triggering reflows.

grafik

staabm commented 6 years ago

another idea on how this could be improved: how about doing the calcuation/init stuff not at construct time but use a IntersectionObserver and do it only for those which are in the currently rendered viewport (or very near to it)?

ganlanyuan commented 6 years ago

el.clientWidth will cause browser recalculations but it has to be like this. Do we have a better solution? For lazy initiating sliders, it's better to do on user (your) side. If I'm going to do this with IntersectionObserver, I have to add a polyfill as well. It will be too much for the slider.

staabm commented 6 years ago

having browser recalculations from el.clientWidth is not the problem, but doing them outside of a requestAnimationFrame is. when doing this kind of things inside a requestAnimationFrame will make the live for the browser easier.

Let the lib user decide on IntersectionObserver is the way to go, you are right. shouldn't be done by the lib itself. good point.

after using the current version from master on my app I can see other code which also is marked as a perf problem:

grafik

ganlanyuan commented 6 years ago

For addCSSRule(), I decided to use it because I don't want to calculate the slide sizes and change their styles every time when clicking the navs or controls. I think it's much better just use CSS to achieve it for one time rather than do it with JavaScript over and over again.

staabm commented 6 years ago

If I understand it correctly the addCSSRule thing would not be a problem when we get it in the correct rendering phase -> requestAnimationFrame.

I would not add another dependency, but a experiment using fastdom could bring in some light as it will separate read/write dom operation and executes them at the "correct time".

https://github.com/wilsonpage/fastdom

ganlanyuan commented 6 years ago

Thanks for the info, will definitely check this recently.

paulvonber commented 6 years ago

@ganlanyuan Is there any news here? as I still got reflow problems with latest version.

ganlanyuan commented 6 years ago

@entwicklerRepo I'm still working on some other small issues. I will check this after those are done.

staabm commented 5 years ago

@ganlanyuan any news?

lindseybradford commented 5 years ago

Just checking in to see if there's any movement on this.