Dev compiler debugger related tweaks.

pkg/dev_compiler/tool/input_sdk/private/ddc_runtime/operations.dart

 // Copyright (c) 2015, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
 /// This library defines runtime operations on objects used by the code
 /// generator.
 part of dart._runtime;
 
 class InvocationImpl extends Invocation {
   final Symbol memberName;
@@ skipping 346 matching lines @@
     (resolvedField) => _callMethod(obj, resolvedField, typeArgs, args, method));
 
 dsendRepl(obj, method, @rest args) => _callMethodRepl(obj, method, null, args);
 
 dgsendRepl(obj, typeArgs, method, @rest args) =>
     _callMethodRepl(obj, method, typeArgs, args);
 
 class _MethodStats {
   final String typeName;
   final String frame;
-  int count;
+  double count;
 
   _MethodStats(this.typeName, this.frame) {
-    count = 0;
+    count = 0.0;
   }
 }
 
+class _CallMethodRecord {
+  var jsError;
+  var type;
+
+  _CallMethodRecord(this.jsError, this.type);
+}
+
 Map<String, _MethodStats> _callMethodStats = new Map();
 
+/// Size for the random sample of dynamic calls.

[vsm, 2017/04/12 21:40:46]

this is perhaps big enough to move out to a separate file?  e.g., profile.dart

[Jacob, 2017/04/12 23:42:09]

Agreed. I was feeling bad about this polluting operations.dart
Moved to profile.dart

+int _callRecordSampleSize = 5000;
+
+/// If the number of dynamic calls exceeds [_callRecordSampleSize] this list
+/// will represent a random sample of the dynamic calls made.
+List<_CallMethodRecord> _callMethodRecords = new List();
+
+/// If the number of dynamic calls exceeds [_callRecordSampleSize] this value
+/// will be greater than [_callMethodRecords.length].
+int _totalCallRecords = 0;
+
+/// Minimum number of samples to consider a profile entry relevant.
+/// This could be set a lot higher. We set this value so users are not
+/// confused into thinking that a dynamic call that occurred once but was
+/// randomly included in the sample is relevant.
+num _minCount = 2;
+
+/// Cache mapping from raw stack frames to source mapped stack frames to
+/// speedup lookup of source map frames when running the profiler.
+/// The number of source map entries looked up makes caching more important
+/// in this case than for typical source map use cases.
+Map<String, String> _frameMappingCache = new Map();
+
 List<List<Object>> getDynamicStats() {
   List<List<Object>> ret = [];
+  // Process the accumulated method stats. This may be quite slow as processing
+  // stack traces is expensive. If there are performance blockers, we should
+  // switch to a sampling approach that caps the number of _callMethodRecords
+  // and uses random sampling to decide whether to add each additional record
+  // to the sample. Main change required is that we need to still show the total
+  // raw number of dynamic calls so that the magnitude of the dynamic call
+  // performance hit is clear to users.
 
+  if (_callMethodRecords.length > 0) {
+    // Ratio between total record count and sampled records count.
+    var recordRatio = _totalCallRecords / _callMethodRecords.length;
+    for (var record in _callMethodRecords) {
+      var stackStr = JS('String', '#.stack', record.jsError);
+      // Truncate the stacktrace as we are only interested in the top 3-4 frames.

[vsm, 2017/04/12 21:40:47]

nit: line len

[Jacob, 2017/04/12 23:42:10]

Done. Fixed comment.

+      var frames = stackStr.split('\n');
+      var src = '';
+      for (int i = 2; i < frames.length; ++i) {
+        var frame = frames[i];
+        var mappedFrame = _frameMappingCache.putIfAbsent(frame, () {
+          return stackTraceMapper('\n${frame}');
+        });
+        if (!mappedFrame.contains(
+            'dart:_internal/js_runtime/lib/ddc_runtime/operations.dart')) {

[vsm, 2017/04/12 21:40:47]

this is a weird uri - are we replacing some prefix with 'dart:' somewhere?

[Jacob, 2017/04/12 23:42:10]

Yeah these are wrong. I need to use libraries.dart to fix these up. Will take a
look at that tomorrow.

+          src = mappedFrame;
+          break;
+        }
+      }
+
+      var actualTypeName = typeName(record.type);
+      _callMethodStats
+          .putIfAbsent("$actualTypeName <$src>",
+              () => new _MethodStats(actualTypeName, src))
+          .count += recordRatio;
+    }
+
+    for (var k in _callMethodStats.keys.toList()) {
+      var stats = _callMethodStats[k];
+      // filter out all calls that did not occur at least _minCount times in the
+      // random sample.
+      var threshold = _minCount * recordRatio;
+      if (stats.count + 0.001 < threshold) {

[vsm, 2017/04/12 21:40:46]

why the 0.001?

[Jacob, 2017/04/12 23:42:09]

epsilon to avoid rounding issues. done.

+        _callMethodStats.remove(k);
+      }
+    }
+  }
+  _callMethodRecords.clear();
+  _totalCallRecords = 0;
   var keys = _callMethodStats.keys.toList();
 
   keys.sort(
       (a, b) => _callMethodStats[b].count.compareTo(_callMethodStats[a].count));
   for (var key in keys) {
     var stats = _callMethodStats[key];
-    ret.add([stats.typeName, stats.frame, stats.count]);
+    ret.add([stats.typeName, stats.frame, stats.count.round()]);
   }
 
   return ret;
 }
 
 clearDynamicStats() {
   _callMethodStats.clear();
+  _callMethodRecords.clear();
 }
 
 bool trackProfile = JS('bool', 'dart.global.trackDdcProfile');
 
 _trackCall(obj) {
   if (JS('bool', '!#', trackProfile)) return;
-
-  var actual = getReifiedType(obj);
-  String stackStr = JS('String', "new Error().stack");
-  var stack = stackStr.split('\n    at ');
-  var src = '';
-  for (int i = 2; i < stack.length; ++i) {
-    var frame = stack[i];
-    if (!frame.contains('dart_sdk.js')) {
-      src = frame;
-      break;
-    }
+  int index = -1;
+  _totalCallRecords++;
+  if (_callMethodRecords.length == _callRecordSampleSize) {
+    // Unfortunately we can't use the excellent Random.nextInt method defined
+    // by Dart from within this library.
+    index = JS('int', 'Math.floor(Math.random() * #)', _totalCallRecords);
+    if (index >= _callMethodRecords.length) return; // don't sample
   }
-
-  var actualTypeName = typeName(actual);
-  _callMethodStats
-      .putIfAbsent(
-          "$actualTypeName <$src>", () => new _MethodStats(actualTypeName, src))
-      .count++;
+  var record =
+      new _CallMethodRecord(JS('', 'new Error()'), getReifiedType(obj));
+  if (index == -1) {
+    _callMethodRecords.add(record);
+  } else {
+    _callMethodRecords[index] = record;
+  }

[vsm, 2017/04/12 21:40:47]

Interesting sampling strategy!  Looks like entries will be somewhat weighted by
time.  E.g., last one wins, right?

[Jacob, 2017/04/12 23:42:09]

Last one doesn't win because we randomly choose to not include each entry in the
sample with the appropriate probability to keep the process fair.

Note this algorithm is conceptually equivalent to shuffling a list of M elements
and then selecting the first N elements of the shuffle.
The only enhancement is I don't bother shuffling until we have > N elements as I
don't care what order those elements occur in as it does not impact the rest of
the algorithm. I omitted that step to make the code simpler to write not because
of the small performance boost.
I'm referring to the standard algorithm to shuffle a list of M elements of
starting with the list in order and then for each index I from 0 to M, randomly
swap that element with the element at 0 <= I with equal probability. My code is
the same as that algorithm except for not bothering to do anything until we are
at element N + 1 and discarding all elements we know will end up at a position >
N.

For fun, here's a slightly hand-waving proof by induction that this approach is
correct:
Base case:
totalCallRecords <= _callRecordSampleSize
Everything is included... so valid "random" sample including everything.

Induction step:
Assume we have a random N record sample of M total records. Each of the original
M records has a probably N/M of being in the sample.

We add record N+1 to the sample with probability N/(M + 1) and do not include it
in the random sample the rest of the time. N / (M + 1) is the correct
probability for an element in the sample of N / (M + 1) elements.
We uniformly randomly select which of the existing N records in the sample to
remove so all records in the existing sample are removed with equal probability
thus their distribution is not skewed. As the new record has the correct
probability of being in the sample, all existing records must have the correct
probability as well although we could calculate it.
Each of the existing records had a probability N/M of being in the sample before
adding element M+1
Each of the existing records has a probability 1 / (M + 1)
of being removed as part of considering element M+1
thus its probability of being in the sample is
(N/M) * (1 - (1/(M+1)) = (N/M) * (M/(M+1)) = N/(M+1)
which is exactly what it should be.

[vsm, 2017/04/13 18:17:49]

Clever!  And makes sense to me.  

Worth a comment here that this approach ensures that each sample has an equal
_callRecordSampleSize/_totalCallRecords chance of inclusion.

 }
 
 /// Shared code for dsend, dindex, and dsetindex.
 _callMethod(obj, name, typeArgs, args, displayName) {
   var symbol = _canonicalMember(obj, name);
   if (symbol == null) {
     return noSuchMethod(
         obj, new InvocationImpl(displayName, args, isMethod: true));
   }
   var f = obj != null ? JS('', '#[#]', obj, symbol) : null;
@@ skipping 556 matching lines @@
     name = '+' + name;
   }
   return name;
 }
 
 /// Emulates the implicit "loadLibrary" function provided by a deferred library.
 ///
 /// Libraries are not actually deferred in DDC, so this just returns a future
 /// that completes immediately.
 Future loadLibrary() => new Future.value();