Greedy script? #17

ZzZombo · 2017-12-21T10:41:34Z

var observer = new MutationObserver(function (mutations) {

        // For each MutationSelectorObserver currently registered.
        for (var j = 0; j < msobservers.length; j++) {
            $(msobservers[j].selector).each(msobservers[j].callback);
        }
    });

If I read the code right, nowhere there the mutations variable is ever referenced. How does the script work then? Does it mean that for all mutations it subscribed to to all elements in the document, it will run callbacks? Huh? That explains the huge performance hit I witness ever since I added the script to my userscript.

The text was updated successfully, but these errors were encountered:

bezborodow · 2018-01-02T04:00:52Z

Hi @ZzZombo. Your performance issue sounds plausible. I'll be looking into it soon.

bezborodow · 2018-01-02T04:02:22Z

For reference: https://developer.mozilla.org/en-US/docs/Web/API/MutationObserver

bezborodow · 2018-01-02T04:15:28Z

There are a few issues here (without delving into the code):

Subscription to all mutations. According to documentation, there are three mutation types: childList, attributes, or characterData. The script needs to observe attributes for situations such as when the class or id attributes are changed. childList must be observed for when a new element is added to the document. We could probably ignore characterData, so we can change that.
The entire document is observed. There is the possibility to add extra options to limit this to specific elements, such as <body> or the main container <div>, etc. This isn't a major issue anyhow.
When a mutation is observed, we search the entire document for matching elements. This could be limited to the element where the mutation occured. However, this could break selectors that use child or descendant combinators. So, this would probably need to be an optional feature to ensure backward compatibility.

ZzZombo · 2018-01-02T04:47:33Z

This shouldn't break anything. Nodes are already inserted/removed by the time an observer handles such actions. Same goes for all the other event types.

pati610 · 2018-02-18T20:58:53Z

@ZzZombo I have 50+ selectors registered, and have no impacts on performance whatsoever. Are you sure that you're only initializing your selectors once? Because if they get reinitialized, let's say at partial loads on a dynamic page, that will choke the page in no time.

I know this from my own experience, because I have to do just that - detect newly inserted DOM elements at every partial ajax page load. The site went crazy when selectors were registered. I wrote a deInitialize func, removing either all or defined selectors, that I attached to a jQuery on hashchange callback. Works great!

@pie6k I'll fork your script and then I'll make a pull request, you are free to incorporate my added func as you see fit (if at all.)

ZzZombo · 2018-02-19T17:21:36Z

The amount of selectors has no impact on performance whatsoever. It's the actual DOM that matters, whether it's a single element "Hello, world" page or both element heavy and dynamically changing one makes huge difference. Using this library in my userscript had actually made my browser ask me if I want to kill unresponsive page or not several times. I no longer use it, having learnt to use the MutationObserver API directly, and while the browser may choke sometimes on complex pages, it still yields better response times overall over the previous version of my script.

bezborodow · 2018-02-20T03:30:22Z

Edit: Deleted to avoid confusion. The content of this post is out-of-date.

bezborodow · 2018-02-20T11:17:28Z

The third feature has also now been implemented in #19.

bezborodow · 2018-02-22T03:08:51Z

Further improvements in #21.

@ZzZombo If you are interested in testing, please test the branch in #21, dbezborodovrp:feature/autoperf

To test via NPM:

npm install github:dbezborodovrp/jquery.initialize#feature/autoperf --save && npm install

ZzZombo · 2018-02-22T03:19:12Z

You made the wrong call. Please read the API specs, or just do a simple test with debug output to browser console. When MutationObserver fires a callback, all changes in the batch it provides have ALREADY executed, as I have said it so in my first response 😩. That's why you are provided with the changed element and optionally **old**-whatever attribute values and not the other way around, element to be changed, and **new**-something values.

bezborodow · 2018-02-22T03:30:01Z

Have you seen and tested the changes?

I'm not sure I follow your reasoning. I know that the element has already been mutated before the observer sees it. That's not a problem. If fact, if the change were not visible, then it would be impossible to query the mutated nodes for matches. So, this seems completely irrelevant to me.

I may have not explained my changes correctly. Disregard the above. I will explain it afresh.

What I have done in #19 and #21 is to address the following concerns:

Does it mean that for all mutations

The plugin now ignores mutation types that are not childList or attributes

it subscribed to to all elements in the document

By default, the plugin will observe all elements (that is, document.documentElement. However, you can now override this by specifying an element. For example:

$.initialize.defaults.target = document.getElementById('observe-this-element');

Or:

$.initialize('div.foo', callbackFunction, { target: document.getElementById('observe-this-element') });

Furthermore, the plugin will not query the entire document for nodes matching the selector. It will only check the mutated node for a match (or its descendants or parents depending upon the complexity of the selector.)

bezborodow · 2018-02-22T03:35:42Z

Did you want to be able to set attributeOldValue to true in the MutationObserverInit options? I can expose that if that is what you are after.

ZzZombo · 2018-02-22T03:35:53Z

$.initialize.defaults.scanDocument = false; is not needed is all I say.

bezborodow · 2018-02-22T03:36:23Z

I agree. I removed that in #21.

ZzZombo · 2018-02-22T03:38:54Z

Then great. Hope to use the updated version in a project soon.

bezborodow · 2018-02-22T03:41:48Z

Very good. The performance improvements haven't been released yet. If you want to test with your application, please do so by installing the test branch:

npm install github:dbezborodovrp/jquery.initialize#feature/autoperf --save && npm install

I also exposed observer options in 573fb61

bezborodow · 2018-04-10T05:42:15Z

These performance changes have been released under 1.2.0.

pie6k added the help wanted label Jan 3, 2018

pati610 mentioned this issue Feb 18, 2018

Deinitialize #18

Closed

bezborodow mentioned this issue Feb 20, 2018

Allow the observed node to be specified and the selector scan search pattern to be configured. #19

Merged

bezborodow closed this as completed Apr 5, 2018

bezborodow added this to the 1.2.0 milestone Apr 10, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Greedy script? #17

Greedy script? #17

ZzZombo commented Dec 21, 2017

bezborodow commented Jan 2, 2018

bezborodow commented Jan 2, 2018

bezborodow commented Jan 2, 2018 •

edited

ZzZombo commented Jan 2, 2018

pati610 commented Feb 18, 2018

ZzZombo commented Feb 19, 2018

bezborodow commented Feb 20, 2018 •

edited

bezborodow commented Feb 20, 2018

bezborodow commented Feb 22, 2018 •

edited

ZzZombo commented Feb 22, 2018 •

edited

bezborodow commented Feb 22, 2018

bezborodow commented Feb 22, 2018

ZzZombo commented Feb 22, 2018

bezborodow commented Feb 22, 2018

ZzZombo commented Feb 22, 2018

bezborodow commented Feb 22, 2018

bezborodow commented Apr 10, 2018 •

edited

Greedy script? #17

Greedy script? #17

Comments

ZzZombo commented Dec 21, 2017

bezborodow commented Jan 2, 2018

bezborodow commented Jan 2, 2018

bezborodow commented Jan 2, 2018 • edited

ZzZombo commented Jan 2, 2018

pati610 commented Feb 18, 2018

ZzZombo commented Feb 19, 2018

bezborodow commented Feb 20, 2018 • edited

bezborodow commented Feb 20, 2018

bezborodow commented Feb 22, 2018 • edited

ZzZombo commented Feb 22, 2018 • edited

bezborodow commented Feb 22, 2018

bezborodow commented Feb 22, 2018

ZzZombo commented Feb 22, 2018

bezborodow commented Feb 22, 2018

ZzZombo commented Feb 22, 2018

bezborodow commented Feb 22, 2018

bezborodow commented Apr 10, 2018 • edited

bezborodow commented Jan 2, 2018 •

edited

bezborodow commented Feb 20, 2018 •

edited

bezborodow commented Feb 22, 2018 •

edited

ZzZombo commented Feb 22, 2018 •

edited

bezborodow commented Apr 10, 2018 •

edited