perf($rootScope): do not mark $watchCollectionInterceptor as $stateful

[jbedard]

I think this was a mistake from the start. This interceptor doesn't actually store any state, it just reads the inner/complex state of its input. EDIT: actually it had a purpose originally, although may no longer be as significant, see #16009 (comment)

Removing the $stateful flag will allow some expressions (those with inputs such as array/object literals) to avoid invoking $watchCollectionInterceptor and avoid creating the literals etc. on each digest.

[gkalpak]

LGTM _{(I think)}

[Narretz]

Can you please add the info from the first post to the commit message? Otherwise LGTM.

[Narretz]

I think this should be fix instead of perf, since it is a prerequisite for #16015 and as you said, probably a mistake from the start

[jbedard]

Updated. This one could probably also go to 1.6.x if there's no objections...

[gkalpak]

I don't have any objections.
_{(I see this more as a perf than fix though, because (hopefully) other that saving some unnecessary work, this change should not in any way affect the behavior of $watchCollection.)}

[jbedard]

I agree it is more of a perf then fix. Even though #16015 depends on it I think that's more of a bug that #16015 fixes, not this. @Narretz any objection to labelling it as perf?

[Narretz]

Fine with me

[jbedard]

FYI I want to look into this more and actually measure it using #16063.

I think I may recall why we originally did this (or why I did it, and didn't leave a comment explaining why...). Essentially because inputs to an expression being used in $watchCollection will often contain non-simple inputs (like collections!). In which case the quick input-watching must fallback to the full expression watching, and might even be a bit slower.

But for literal expressions it sure would be nice if this flag was off since shallow-watching the inputs is all we need to do, and we can avoid constructing the literal and analyzing the collection with this interceptor etc.

[jbedard]

After a bit more investigation, just removing $stateful to allow input watching will...

• add a minor overhead to everything (due to the inputsWatchDelegate)
• may remove the overhead of $watchCollectionInterceptor when all inputs are simple values
• may remove the major overhead of reconstructing literals when all inputs are simple values

I've updated the first commit to only remove $stateful for literals since non-literals will never get the benefits but always get that extra minor overhead.

I've also added a second commit which adds the (internal) concept of $pure interceptors in addition to the existing $stateful concept. If an interceptor is $pure then it will only get invoked if it's inputs have changed, where normally it is invoked if inputs changed or those inputs are non-primitive values. This does not add it to filters or any public interface but that is a possibility for the future (and is like the Angular pure flag).

Locally the parsed-expressions-bp $watchCollection literal tests are 2x as fast and GC went from very-high to 0 when the literal reconstructing is avoided.

I think adding $pure is worth it since it allows this perf gain for all literals (not only those with only simple inputs). WDYT?

[jbedard]

Split into 2 lines

[jbedard]

Change to $$pure until documented

[jbedard]

Some benchmarks... when running the parsed-expressions-bp $watchCollection Literals test (basically $watchCollection on those expressions 10000x):

Old: ~5.2s, >375000kb GCed
New: ~2.1s, <10kb GCed

[jbedard]

I think travis passed now, but github hasn't picked up the new status?

[Narretz]

The problem with Travis is that the deploy job isn't automatically started after one of the test jobs fails and then passes. I've restarted it manually