marionette.inspector icon indicating copy to clipboard operation
marionette.inspector copied to clipboard

Investigate watch.js `loop` perf issues

Open jasonLaster opened this issue 9 years ago • 5 comments

Problem

We use watch.js to observe changes to objects like Model attributes or Radio channels. The way, watch.js observes changes to objects lengths is by checking their length every 50ms. This happens in watch.js loop which loops over all of the lengthSubjects.

When we run the chrome devtools cpu profile over the code, we find that the loop function inside of watch.js takes a significant amount of time to diff the objects and see if they've changed. Maybe there are easy ways to either remove listeners or call loop at better times for the cpu.

jasonLaster avatar Dec 03 '14 02:12 jasonLaster

But Angular thinks it's a good idea!

cmaher avatar Dec 03 '14 13:12 cmaher

Just looked into this again and found two interesting things:

  1. loop is expensive (see all of the functions that refer to loop in the perf
  2. We're actually doing a lot to make loop more performant. the lazyWorker + 500ms implementation is good. I wonder what else we can do there

  setInterval(function() {
        if(!__agent) return

        __agent.lazyWorker.push({
          context: this,
          args: [],
          callback: loop
        })

    }, 500);

jasonLaster avatar Mar 12 '15 19:03 jasonLaster

Follow up:

@ianmstew found that callWatchers can be slow - in one example 125ms Here's the code so you know what I'm talking about. Callwatchers is the methods we call for each object to check to seee if anyything has changed.

We should probably do one of two things:

  • batch loop so that we only check so many items at once
  • remove items that we're watching once they're destroyed or removed from the dom. the easiest would be views...

Of the two, I prefer batch as that's the easiest short term fix

jasonLaster avatar Apr 14 '15 01:04 jasonLaster

Here's a look at one instance, 115ms, where callWatchers is executed 3 times consecutively immediately following ajax success and immediately before view rendering. Notice how it appears on the graph as a mostly flat line. There are other instances of this in the graph, but the extra delay between ajax success and view rendering likely affects perception of the Inspector's "snappiness".

screen shot 2015-04-13 at 10 35 38 pm

ianmstew avatar Apr 14 '15 02:04 ianmstew

wow - i think that should be fixed now @ianmstew w/ the better delay, but man one feature of the lazyWorker would be a de-dupe function.

currently, new loops could be added to the worker queue and it doesn't care. We should be able to say, if you see this task w/ these params again, don't do the first or the last...

jasonLaster avatar Apr 17 '15 14:04 jasonLaster