feat(WatchGroup): Allow measuring cost of running reaction functions and #1148

mvuksano · 2014-06-12T14:09:36Z

To be able to measure cost of reaction function, dirty check and/or
dirty watch one needs to define ExecutionStats. ExecutionStats is
configured using ExecutionStatsConfig object. This object can be used to
define how many records to display when showing the stats, what's the
minimum threshold (in microseconds) for a record to be considered as
relevant as well as to enable and disable tracking the stats.

One can define ExecutionStats, ExectionStatsEmitter and
ExecutionStatsConfig like:

  m..bind(ExectionStats)
   ..bind(ExectionStatsEmitter)
   ..bind(ExectionStatsConfig, toFactory: (i) => new ExectionStats(...))

The results can be displayed using ngStats object. To obtain a reference
to an ngStats object type (in the console):

  ng = ngStats();

To display maxNumber (defined using ExectionStatsConfig object) of the
most expensive reaction functions type:

  ng.showReactionFnStats();

To display maxNumber of most expensive evals type:

  ng.showEvalStats();

To display maxNumber of most expensive dirty checks type:

  ng.showDirtyCheckStats();

One can use ng.enable() method to enable tracking and ng.disable()
method to disable trackin. To clear the results use ng.reset()
method.

To achieve least performance impact on the application make sure
that the ExectionStats is null.

  m..bind(ExectionStats, toValue: null);

That way no extra objects will be created and the impact on the
actual application will be minimal.

IgorMinar · 2014-06-12T15:46:03Z

lib/change_detection/watch_group.dart

+  void addTime(ReactionFnStatsEntry entry) {
+    try {
+      stats[count++] = entry;
+    } on RangeError {


using exceptions for control flow? why not check if stats length is lower than count++?

yes, exceptions are very expensive. Change to range check.

IgorMinar · 2014-06-12T15:50:55Z

I don't know enough about the previous conversations about this feature, but the current implementation looks pretty heavy weight.

wouldn't a sufficient goal be to report just two number - the total cost of reactions per digest and total number of reactions per digest? we could then report this along with dirty checking and flush.

jeffbcross · 2014-06-13T16:13:22Z

Why aren't there any tests included with this PR?

jeffbcross · 2014-06-13T21:00:12Z

Where is ReactionFnStats defined?

mvuksano · 2014-06-13T21:03:34Z

@jeffbcross I'm still refactoring this. It's actually been renamed to ExecutionStats. I should be done with refactorings soon.

jeffbcross · 2014-06-13T21:04:52Z

Great! Are you adding tests as you refactor?

mvuksano · 2014-06-13T21:15:15Z

Yes, I will do that. Is there any specific behaviour that you'd like to see how to use? Or was this just a general question?

jeffbcross · 2014-06-13T21:27:14Z

A few reasons for asking about tests:

I'd like to look at tests to see your code in action, and how I can use it
I'd like to know that everything works before I try to integrate it
We generally require tests for all new angular functionality, even utilities like this (but I'm a noob on angular.dart, and maybe there's a reason why tests aren't required for this; I defer to @mhevery)

mvuksano · 2014-06-13T22:21:55Z

lib/change_detection/execution_stats.dart

+  void showEvalStats(ExecutionStats fnStats) {
+    _printLine('Time (us)', 'Fn/Name');
+    fnStats.getEvalStats().forEach((ExecutionEntry entry) {
+      var text = (entry.value as EvalWatchRecord).fn != null ?


@mhevery What would be useful information to display about EvalWatchRecord?

Actually, figured out that EvalWatchRecord.toString will do just fine.

mvuksano · 2014-06-13T22:25:18Z

@mhevery Could you have a look at this. There's still some more work to be done here but feedback would be useful. I'm doubt that extracting info from EvalWatchRecord using mirrors is a good idea. If not, what's your suggestion?

mvuksano · 2014-06-13T22:42:11Z

Mirrors seems like a bad idea... They will increase the size of compiled code...

vicb · 2014-06-15T13:49:33Z

lib/change_detection/execution_stats.dart

+
+  Iterable<ExecutionEntry> getReactionFnStats() {
+    _shrinkDirtyWatch();
+    return dirtyWatchStats.getRange(0, _capacity);


should all the get<...>() be true getters ?

reporting it. To be able to measure cost of reaction function, dirty check and/or dirty watch one needs to define ExecutionStats. ExecutionStats is configured using ExecutionStatsConfig object. This object can be used to define how many records to display when showing the stats, what's the minimum threshold (in microseconds) for a record to be considered as relevant as well as to enable and disable tracking the stats. One can define ExecutionStats, ExectionStatsEmitter and ExecutionStatsConfig like: m..bind(ExectionStats) ..bind(ExectionStatsEmitter) ..bind(ExectionStatsConfig, toFactory: (i) => new ExectionStats(...)) The results can be displayed using ngStats object. To obtain a reference to an ngStats object type (in the console): ng = ngStats(); To display maxNumber (defined using ExectionStatsConfig object) of the most expensive reaction functions type: ng.showReactionFnStats(); To display maxNumber of most expensive evals type: ng.showEvalStats(); To display maxNumber of most expensive dirty checks type: ng.showDirtyCheckStats(); One can use ng.enable() method to enable tracking and ng.disable() method to disable trackin. To clear the results use ng.reset() method. To achieve least performance impact on the application make sure that the ExectionStats is null. m..bind(ExectionStats, toValue: null); That way no extra objects will be created and the impact on the actual application will be minimal.

mhevery · 2014-06-16T18:55:48Z

lib/change_detection/dirty_checking_change_detector.dart

@@ -312,7 +315,19 @@ class DirtyCheckingChangeDetector<H> extends DirtyCheckingChangeDetectorGroup<H>
    int count = 0;
    while (current != null) {
      try {
-        if (current.check()) changeTail = changeTail._nextChange = current;
+        if (executionStats != null && executionStats.config.enabled) {


I think a better way to do this is to duplicate the whole while loop. One while loop is instrumented and the other is not.

The stats objects should be injected in the constructor and should have on off switch which would then control which while loop to chose. That way one could turn on/off the stats during the lifetime.

mhevery · 2014-07-21T23:38:51Z

@chirayuk Do we still need this or does your change supersedes this one?

mhevery · 2014-08-26T21:40:56Z

Superseded by @chirayuk's wor.

mvuksano added cla: yes and removed cla: no labels Jun 12, 2014

IgorMinar reviewed Jun 12, 2014
View reviewed changes

mvuksano reviewed Jun 13, 2014
View reviewed changes

vicb reviewed Jun 15, 2014
View reviewed changes

mhevery reviewed Jun 16, 2014
View reviewed changes

jbdeboer added the Performance label Jun 25, 2014

mhevery added the state: reviewing label Jul 23, 2014

jbdeboer added the state: rebase label Aug 25, 2014

mhevery closed this Aug 26, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(WatchGroup): Allow measuring cost of running reaction functions and #1148

feat(WatchGroup): Allow measuring cost of running reaction functions and #1148

mvuksano commented Jun 12, 2014

IgorMinar Jun 12, 2014

mhevery Jun 12, 2014

IgorMinar commented Jun 12, 2014

jeffbcross commented Jun 13, 2014

jeffbcross commented Jun 13, 2014

mvuksano commented Jun 13, 2014

jeffbcross commented Jun 13, 2014

mvuksano commented Jun 13, 2014

jeffbcross commented Jun 13, 2014

mvuksano Jun 13, 2014

mvuksano Jun 13, 2014

mvuksano commented Jun 13, 2014

mvuksano commented Jun 13, 2014

vicb Jun 15, 2014

mhevery Jun 16, 2014

mhevery commented Jul 21, 2014

mhevery commented Aug 26, 2014

feat(WatchGroup): Allow measuring cost of running reaction functions and #1148

feat(WatchGroup): Allow measuring cost of running reaction functions and #1148

Conversation

mvuksano commented Jun 12, 2014

IgorMinar Jun 12, 2014

Choose a reason for hiding this comment

mhevery Jun 12, 2014

Choose a reason for hiding this comment

IgorMinar commented Jun 12, 2014

jeffbcross commented Jun 13, 2014

jeffbcross commented Jun 13, 2014

mvuksano commented Jun 13, 2014

jeffbcross commented Jun 13, 2014

mvuksano commented Jun 13, 2014

jeffbcross commented Jun 13, 2014

mvuksano Jun 13, 2014

Choose a reason for hiding this comment

mvuksano Jun 13, 2014

Choose a reason for hiding this comment

mvuksano commented Jun 13, 2014

mvuksano commented Jun 13, 2014

vicb Jun 15, 2014

Choose a reason for hiding this comment

mhevery Jun 16, 2014

Choose a reason for hiding this comment

mhevery commented Jul 21, 2014

mhevery commented Aug 26, 2014