Complete the switch to ServiceObject

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

+// Copyright (c) 2014, 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.
+
+part of service;
+
+/// Abstract [ServiceObjectCache].
+abstract class ServiceObjectCache<T extends ServiceObject> {
+  final Isolate isolate;
+  final _cache = new ObservableMap<String, T>();
+
+  ServiceObjectCache(this.isolate) {
+    assert(isolate != null);
+  }
+
+  /// Returns true if [this] caches objects with this [id].
+  bool cachesId(String id);

[turnidge, 2014/03/10 21:03:28]

What do you think of blank lines between these decls?  I would find it less
dense.

+  /// Returns true if [this] caches objects with service [type].
+  bool cachesType(String type);

[turnidge, 2014/03/10 21:03:28]

I'm not convinced that we want to have a data-driven approach to dispatching to
the appropriate cache.  I don't see the benefit yet.

+  /// Upgrades [obj] into a [T].

[turnidge, 2014/03/10 21:03:28]

More verbose comment about what "upgrading" means.

+  T upgrade(ObservableMap map);

[turnidge, 2014/03/10 21:03:28]

Could upgrade be private (_upgrade) or is it part of the public api of a cache?

[Cutch, 2014/03/11 03:17:48]

Done.

+
+  /// Returns true if [this] has [id] in its cache.
+  bool contains(String id) {
+    assert(cachesId(id));
+    return _cache[id] != null;
+  }
+
+  /// Gets [id] from the cache. Returns null if not contained.
+  T operator[](String id) {
+    assert(cachesId(id));
+    return _cache[id];
+  }
+
+  /// Caches [so] with [id].
+  operator[]=(String id, T so) {

[turnidge, 2014/03/10 21:03:28]

"so" -> obj?

[Cutch, 2014/03/11 03:17:48]

Done.

+    assert(cachesId(id));
+    assert(cachesType(so.serviceType));
+    _cache[id] = so;
+  }
+
+  /// Gets [id] from the cache or makes a network request for [id].
+  Future<T> get(String id) {
+    assert(cachesId(id));
+    T cached = _cache[id];
+    if (cached != null) {
+      return new Future.value(cached);
+    }
+    return isolate.fetch(id).then(_addToCache);
+  }
+
+  /// If [obj] is cached, return the cached object. Otherwise, upgrades [obj]
+  /// and adds the upgraded value to the cache.
+  T cachedOrUpgradeAndAdd(ObservableMap obj) {

[turnidge, 2014/03/10 21:03:28]

I would like a different name.  Maybe something like "putIfAbsent".  When does
this get used -- cause I would like to look at the use of the function and
puzzle it out a bit.

[Cutch, 2014/03/11 03:17:48]

I've renamed it to putIfAbsent. This gets called by the ServiceObject factory:
_upgradeToServiceObject

+    assert(ServiceObject.isServiceMap(obj));
+    String id = obj['id'];
+    var type = obj['type'];
+    assert(cachesId(id));
+    assert(cachesType(type));
+    if (contains(id)) {
+      return this[id];
+    }
+    return _addToCache(upgrade(obj));
+  }
+
+  T _addToCache(T so) {
+    this[so.id] = so;
+    return so;
+  }
+}
+
+class ScriptCache extends ServiceObjectCache<Script> {
+  ScriptCache(Isolate isolate) : super(isolate);
+
+  bool cachesId(String id) => _matcher.hasMatch(id);
+  bool cachesType(String type) => ServiceObject.unreffedType(type) == 'Script';

[turnidge, 2014/03/10 21:03:28]

unreffedType... the double f is weird.  Maybe "stripRef"?

[Cutch, 2014/03/11 03:17:48]

Done.

+  Script upgrade(ObservableMap obj) => new Script.fromMap(isolate, obj);
+  static final RegExp _matcher = new RegExp(r'scripts/.+');
+
+  void _processCoverage(ServiceMap coverage) {
+    assert(coverage.serviceType == 'CodeCoverage');
+    var coverageList = coverage['coverage'];
+    assert(coverageList != null);
+    coverageList.forEach((scriptCoverage) {
+      _processScriptCoverage(scriptCoverage);
+    });
+  }
+
+  void _processScriptCoverage(ObservableMap scriptCoverage) {
+    // Because the coverage data was upgraded into a ServiceObject,
+    // the script can be directly accessed.
+    Script script = scriptCoverage['script'];
+    script._processHits(scriptCoverage['hits']);
+  }
+}
+
+class CodeCache extends ServiceObjectCache<Code> {
+  CodeCache(Isolate isolate) : super(isolate);
+
+  bool cachesId(String id) => _matcher.hasMatch(id);
+  bool cachesType(String type) => ServiceObject.unreffedType(type) == 'Code';
+  Code upgrade(ObservableMap obj) => new Code.fromMap(isolate, obj);
+
+  static final RegExp _matcher = new RegExp(r'code/.+');
+
+  List<Code> topExclusive(int count) {
+    var codeList = _cache.values.toList();
+    codeList.sort((Code a, Code b) {
+      return b.exclusiveTicks - a.exclusiveTicks;
+    });
+    if (codeList.length < count) {
+      return codeList;
+    }
+    codeList.length = count;
+    return codeList;
+  }
+
+  void _resetProfileData() {
+    _cache.forEach((k, Code code) {
+      code.resetProfileData();
+    });
+  }
+
+  void _updateProfileData(ServiceMap profile, List<Code> codeTable) {
+    var codes = profile['codes'];
+    var sampleCount = profile['samples'];
+    for (var profileCode in codes) {
+      Code code = profileCode['code'];
+      code.updateProfileData(profileCode, codeTable, sampleCount);
+    }
+  }
+}
+
+class ClassCache extends ServiceObjectCache<ServiceMap> {
+  ClassCache(Isolate isolate) : super(isolate);
+
+  bool cachesId(String id) => _matcher.hasMatch(id);
+  bool cachesType(String type) => ServiceObject.unreffedType(type) == 'Class';
+  ServiceMap upgrade(ObservableMap obj) => new ServiceMap.fromMap(isolate, obj);
+
+  static final RegExp _matcher = new RegExp(r'classes/\d+$');
+}