que-etc / resize-observer-polyfill

A polyfill for the Resize Observer API
MIT License
1.75k stars 132 forks source link

Question about performance #7

Open callumlocke opened 7 years ago

callumlocke commented 7 years ago

This polyfill looks great. I'm trying to decide whether it's worth using on my project.

I would appreciate a bit more context for this:

Implementation is based on the MutationObserver (no polling unless DOM changes) with a fall back to a continuous dirty checking cycle if the first one is not supported.

que-etc commented 7 years ago

@callumlocke, thanks for your interest. I'll go straight to answering your questions:

Am I right in assuming the MutationObserver method will work on all the green browsers on caniuse?

Yes, except that this library doesn't use MO when running in IE 11 because of the pitfall described in the issue #6 (try running this code in IE 11). On top of the caniuse list, MutationObserver works in:

MutationObserver standalone doesn't cause any visible performance degradation: it doesn't interrupt the reflow/repaint cycles of a browser and, unlike events, it batches changes and doesn't fire notifications right on the spot. So, it's all about what's happening on the MutationCallback's side, e.g. making a stacktrace of DOM changes will definitely slower down your app and increase memory consumption, but luckily this is not our case :)

As for comparison with other related approaches:

que-etc commented 7 years ago

@callumlocke well, I've finally added this semblance of a benchmark test: https://jsfiddle.net/que_etc/gaqLe8rn/

Can't really say if its results are relevant but they show a neglectable difference with MutationObserver slowing down the suite by < 1%.

que-etc commented 7 years ago

Added a test case which utilizes Mutation Events as I've been thinking of completely replacing polling with them. It's peculiar that they outperform MutationObserver when used in IE11.

trusktr commented 7 years ago

@que-etc This is an awesome polyfill.

How does this compare to a poll loop using requestAnimationFrame? In my project, I'm hooking into my engine's loop and using getComputedStyle on a MotorHTMLScene's parentElement.

Basically like this:

// ...

    _startSizePolling() {
        // NOTE Polling is currently required because there's no other way to do this
        // reliably, not even with MutationObserver. ResizeObserver hasn't
        // landed in browsers yet.
        if (!this._sizePollTask)
            this._sizePollTask = Motor.addRenderTask(this._checkSize.bind(this))
    }

// ...

    _checkSize() {

        // The scene has a parent by the time this is called (see
        // src/motor/Scene#mount where _startSizePolling is called)
        const parent = this.parentNode
        const parentSize = this._parentSize
        const style = getComputedStyle(parent)
        const width = parseFloat(style.width)
        const height = parseFloat(style.height)

        // if we have a size change, trigger parentsizechange
        if (parentSize.x != width || parentSize.y != height) {
            parentSize.x = width
            parentSize.y = height

            this.triggerEvent('parentsizechange', Object.assign({}, parentSize))
        }
    }

// ...

Motor.addRenderTask(this._checkSize.bind(this)) is a layer on top of requestAnimationFrame, so it's similar to requestAnimationFrame(this._checkSize.bind(this)).

How does your technique compare to mine? Thoughts on which performs better?

I know that with my method the size is guaranteed to be accurate all the time. Are there any edge cases for yours? For example, can you briefly explain: how does yours work with CSS transitions? i.e. How does MutationObserver work with CSS transitions? I thought MO was observing element content and attribute changes (f.e. an MO callback can look at changes to the styleattribute), but how can MO get a change for each frame of a CSS transition?

Maybe due to my limited knowledge, it seems like the only way to detect size changes in CSS transitions is with polling.

trusktr commented 7 years ago

Hehe, notice my comment,

        // NOTE Polling is currently required because there's no other way to do this
        // reliably, not even with MutationObserver. ResizeObserver hasn't
        // landed in browsers yet.

I might be wrong if you have this polyfill working with MutationObserver, I just don't see how yet.

nikparo commented 7 years ago
how does yours work with CSS transitions?

It is not supposed to notice transitions though, is it? At least the current draft (https://wicg.github.io/ResizeObserver/) specifies that "observations will not be triggered by CSS transforms."

trusktr commented 7 years ago

observations will not be triggered by CSS transforms

Maybe they are saying that the transform property doesn't affect sizing (which makes sense). Had the word been "transitions" then I'd be worried. If it doesn't work with CSS transitions, then that'd be no good!

nikparo commented 7 years ago

Ah it seems I misread, my bad!

adamscybot commented 6 years ago

Just adding a note here for anyone googling. This package caused a very hard to find memory leak in IE11. This is obviously IE's fault, not the package. But it seems that if you have a tab open a long time with many page reloads (full page reloads -- which should obviously clear memory in every scenario!), memory use gets critical and the page crashes.

This seems to be an issue with IE11's MutationObserver API. I used webpack DefinePlugin to fool this lib into thinking MutationObserver was not available and the problem went away.