Add isolate tag-profile and better handling of errors

runtime/bin/vmservice/client/lib/src/service/object.dart

Index: runtime/bin/vmservice/client/lib/src/service/object.dart
diff --git a/runtime/bin/vmservice/client/lib/src/service/object.dart b/runtime/bin/vmservice/client/lib/src/service/object.dart
index 0a2129f692a9521410be796be8fdf5f8492db8c6..bda1de23f920699bf441ac9c01e99db5bb6afe06 100644
--- a/runtime/bin/vmservice/client/lib/src/service/object.dart
+++ b/runtime/bin/vmservice/client/lib/src/service/object.dart
@@ -102,10 +102,6 @@ abstract class ServiceObject extends Observable {
   ServiceObject update(ObservableMap m) {
     // Assert that m is a service map.
     assert(ServiceObject.isServiceMap(m));
-    if ((m['type'] == 'Error') && (_serviceType != 'Error')) {
-      // Got an unexpected error. Don't update the object.
-      return _upgradeToServiceObject(vm, isolate, m);
-    }
     // TODO(johnmccutchan): Should we allow for a ServiceObject's id
     // or type to change?
     _id = m['id'];
@@ -118,9 +114,6 @@ abstract class ServiceObject extends Observable {
   void _update(ObservableMap map);
 
   void _created() {
-    var refNotice = _ref ? ' Created from reference.' : '';
-    Logger.root.info('Created ServiceObject for \'${_id}\' with type '
-                     '\'${_serviceType}\'.' + refNotice);
   }

[turnidge, 2014/03/24 20:22:13]

Maybe just drop _created() entirely.

[Cutch, 2014/03/25 14:39:06]

Done.

 
   // ------------------------------------------------------
@@ -171,6 +164,11 @@ abstract class VM extends ServiceObject {
     _initOnce();
   }
 
+  final StreamController<ServiceException> exceptions =
+      new StreamController.broadcast();
+  final StreamController<ServiceError> errors =
+      new StreamController.broadcast();
+
   static final RegExp _currentIsolateMatcher = new RegExp(r'isolates/\d+');
   static final RegExp _currentObjectMatcher = new RegExp(r'isolates/\d+(/|$)');
 
@@ -222,30 +220,53 @@ abstract class VM extends ServiceObject {
     }
   }
 
-  /// Gets [id] as an [ObservableMap] from the service directly.
+  /// Gets [id] as an [ObservableMap] from the service directly. If
+  /// an error occurs, the future is completed as an error with a
+  /// ServiceError or ServiceException. Therefore the chained then()
+  /// execution path will always receive a map encoding a ServiceObject.
   Future<ObservableMap> getAsMap(String id) {
     return getString(id).then((response) {
       try {
-        var map = JSON.decode(response);
-        Logger.root.info('Decoded $id');
-        Logger.root.info('Response $response');
-        return toObservable(map);
+        var map = toObservable(JSON.decode(response));
+        if (!ServiceObject.isServiceMap(map)) {
+          return new Future.error(
+                new ServiceException.fromMap(this, toObservable({
+            'type': 'ServiceException',
+            'id': '',
+            'kind': 'FormatException',
+            'response': map,
+            'message': 'Top level service responses must be service maps.',
+          })));
+        }
+        // Preemptively capture ServiceError and ServiceExceptions.
+        if (map['type'] == 'ServiceError') {
+          return new Future.error(new ServiceError.fromMap(this, map));
+        } else if (map['type'] == 'ServiceException') {
+          return new Future.error(new ServiceException.fromMap(this, map));
+        }
+        // map is now guaranteed to be a non-error/exception ServiceObject.
+        return map;
       } catch (e, st) {
-        return toObservable({
-          'type': 'Error',
+        print(e);
+        print(st);
+        return new Future.error(
+              new ServiceException.fromMap(this, toObservable({
+          'type': 'ServiceException',
           'id': '',
-          'kind': 'DecodeError',
-          'message': '$e',
-        });
+          'kind': 'DecodeException',
+          'response': response,
+          'message': 'Could not decode JSON: $e',
+        })));
       }
     }).catchError((error) {
-      return toObservable({
-        'type': 'Error',
-        'id': '',
-        'kind': 'LastResort',
-        'message': '$error'
-      });
-    });
+      // ServiceError.
+      errors.add(error);
+      return new Future.error(error);
+    }, test: (e) => e is ServiceError).catchError((exception) {

[turnidge, 2014/03/24 20:22:13]

I hadn't seen the "test" stuff before.  Wacky!

+      // ServiceException.
+      exceptions.add(exception);
+      return new Future.error(exception);
+    }, test: (e) => e is ServiceException);
   }
 
   /// Get [id] as a [String] from the service directly. See [getAsMap].
@@ -262,8 +283,92 @@ abstract class VM extends ServiceObject {
   }
 }
 
+class TagProfileSnapshot {
+  final double seconds;
+  final List<int> counters;
+  int get sum => _sum;
+  int _sum = 0;
+  TagProfileSnapshot(this.seconds, int countersLength)
+      : counters = new List<int>(countersLength);
+
+  /// Set [counters] and update [sum].
+  void set(List<int> counters) {
+    this.counters.setAll(0, counters);
+    for (var i = 0; i < this.counters.length; i++) {
+      _sum += this.counters[i];
+    }
+  }
+
+  /// Set [counters] with the delta from [counters] to [old_counters]
+  /// and update [sum].
+  void delta(List<int> counters, List<int> old_counters) {
+    for (var i = 0; i < this.counters.length; i++) {
+      this.counters[i] = counters[i] - old_counters[i];
+      _sum += this.counters[i];
+    }
+  }
+
+  /// Update [counters] with new maximum values seen in [counters].
+  void max(List<int> counters) {
+    for (var i = 0; i < counters.length; i++) {
+      var c = counters[i];
+      this.counters[i] = this.counters[i] > c ? this.counters[i] : c;
+    }
+  }
+
+  /// Zero [counters].
+  void zero() {
+    for (var i = 0; i < counters.length; i++) {
+      counters[i] = 0;
+    }
+  }
+}
+
+class TagProfile {
+  final List<String> names = new List<String>();
+  final List<TagProfileSnapshot> snapshots = new List<TagProfileSnapshot>();
+  double get updatedAtSeconds => _seconds;
+  double _seconds;
+  TagProfileSnapshot _maxSnapshot;
+  int _historySize;
+  int _countersLength = 0;
+
+  TagProfile(this._historySize);
+
+  void _processTagProfile(double seconds, ObservableMap tagProfile) {
+    _seconds = seconds;
+    var counters = tagProfile['counters'];
+    if (names.length == 0) {
+      // Initialization.
+      names.addAll(tagProfile['names']);
+      _countersLength = tagProfile['counters'].length;
+      for (var i = 0; i < _historySize; i++) {
+        var snapshot = new TagProfileSnapshot(0.0, _countersLength);
+        snapshot.zero();
+        snapshots.add(snapshot);
+      }
+      // The counters monotonically grow, keep track of the maximum value.
+      _maxSnapshot = new TagProfileSnapshot(0.0, _countersLength);
+      _maxSnapshot.set(counters);
+      return;
+    }
+    var snapshot = new TagProfileSnapshot(seconds, _countersLength);
+    // We snapshot the delta from the current counters to the maximum counter
+    // values.
+    snapshot.delta(counters, _maxSnapshot.counters);
+    _maxSnapshot.max(counters);
+    snapshots.add(snapshot);
+    // Only keep _historySize snapshots.
+    if (snapshots.length > _historySize) {
+      snapshots.removeAt(0);
+    }
+  }
+}

[turnidge, 2014/03/24 20:22:13]

What do you think of us moving the profilng helper classes out of object.dart
later?  Not for this review - would conflict too horribly with my change.

[Cutch, 2014/03/25 14:39:06]

sgtm.

+
 /// State for a running isolate.
 class Isolate extends ServiceObject {
+  final TagProfile tagProfile = new TagProfile(20);
+
   String get link => _id;
   String get hashLink => '#/$_id';
 
@@ -372,6 +477,8 @@ class Isolate extends ServiceObject {
 
   @observable String fileAndLine;
 
+  @observable DartError stickyError;
+
   void _update(ObservableMap map) {
     upgradeCollection(map, vm, this);
     mainPort = map['mainPort'];
@@ -420,6 +527,15 @@ class Isolate extends ServiceObject {
     pausedOnExit = map['pausedOnExit'];
     running = map['topFrame'] != null;
     idle = !pausedOnStart && !pausedOnExit && !running;
+    stickyError = map['stickyError'];
+  }
+
+  Future<TagProfile> updateTagProfile() {
+    return vm.getAsMap(relativeLink('profile/tag')).then((ObservableMap m) {
+      var seconds = new DateTime.now().millisecondsSinceEpoch / 1000.0;
+      tagProfile._processTagProfile(seconds, m);
+      return tagProfile;
+    });
   }
 
   @reflectable CodeTrieNode profileTrieRoot;
@@ -474,7 +590,7 @@ class IsolateList {
   final isolates = new ObservableMap<String, Isolate>();
 
   IsolateList(this._vm);
-  
+
   void updateIsolates(List<Map> members) {
      // Find dead isolates.
      var deadIsolates = [];
@@ -545,7 +661,6 @@ class IsolateList {
   }
 }
 
-
 /// A [ServiceObject] which implements [ObservableMap].
 class ServiceMap extends ServiceObject implements ObservableMap {
   final ObservableMap _map = new ObservableMap();
@@ -598,8 +713,42 @@ class ServiceMap extends ServiceObject implements ObservableMap {
   bool get hasObservers => _map.hasObservers;
 }
 
+/// A [DartError] is peered to a Dart Error object.
+class DartError extends ServiceObject {
+  DartError.fromMap(ServiceObject owner, Map m) : super.fromMap(owner, m);
+
+  @observable String kind;
+  @observable String message;
+  @observable ServiceMap exception;
+  @observable ServiceMap stacktrace;
+
+  void _update(ObservableMap map) {
+    kind = map['kind'];
+    message = map['message'];
+    if (map['exception'] != null) {
+      exception = new ServiceMap.fromMap(owner, map['exception']);
+    } else {
+      exception = null;
+    }
+    if (map['stacktrace'] != null) {
+      stacktrace = new ServiceMap.fromMap(owner, map['stacktrace']);
+    } else {
+      stacktrace = null;
+    }
+    name = 'DartError $kind';
+    vmName = name;
+  }
+
+  // TODO: stackTrace?

[turnidge, 2014/03/24 20:22:13]

Delete this TODO?

[Cutch, 2014/03/25 14:39:06]

Done.

+}
+
+/// A [ServiceError] is an error that was triggered in the service
+/// server or client. Errors are prorammer mistakes that could have
+/// been prevented, for example, requesting a non-existant path over the
+/// service.
 class ServiceError extends ServiceObject {
-  ServiceError.fromMap(ServiceObject owner, Map m) : super.fromMap(owner, m);
+  ServiceError.fromMap(ServiceObject owner, Map m)
+      : super.fromMap(owner, m);
 
   @observable String kind;
   @observable String message;
@@ -610,8 +759,26 @@ class ServiceError extends ServiceObject {
     name = 'ServiceError $kind';
     vmName = name;
   }
+}
 
-  // TODO: stackTrace?
+/// A [ServiceException] is an exception that was triggered in the service
+/// server or client. Exceptions are events that should be handled,
+/// for example, an isolate went away or the connection to the VM was lost.
+class ServiceException extends ServiceObject {
+  ServiceException.fromMap(ServiceObject owner, Map m)
+      : super.fromMap(owner,  m);
+
+  @observable String kind;
+  @observable String message;
+  @observable dynamic response;
+
+  void _update(ObservableMap map) {
+    kind = map['kind'];
+    message = map['message'];
+    response = map['response'];
+    name = 'ServiceException $kind';
+    vmName = name;
+  }
 }
 
 class ScriptLine {
@@ -884,7 +1051,11 @@ class Code extends ServiceObject {
     startAddress = int.parse(m['start'], radix:16);
     endAddress = int.parse(m['end'], radix:16);
     function = _upgradeToServiceObject(vm, isolate, m['function']);
-    objectPool = _upgradeToServiceObject(vm, isolate, m['object_pool']);
+    if (m['object_pool'] != null) {
+      objectPool = _upgradeToServiceObject(vm, isolate, m['object_pool']);
+    } else {
+      objectPool = null;
+    }
     var disassembly = m['disassembly'];
     if (disassembly != null) {
       _processDisassembly(disassembly);