[dart2js] Use Indexed{Sink/Source} for ConstantValue

rakudrama · commit-bot@chromium.org · commit 9d786640df53 · 2019-07-24T17:12:43.000Z
This avoids naive tree-expansion of constant DAGs. Fixes a bug in IndexedSink where multiple nodes in a tree or list can be written as the same index, so on reading the second node is returned instead of the first. This affects source information of inlined methods where some frames are dropped. The .map files are about 10% larger with the frames. Change-Id: I7e39c72510145a5ee52ea07ad2a3860bd382ee20 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/110181 Reviewed-by: Johnni Winther <johnniwinther@google.com> Commit-Queue: Stephen Adams <sra@google.com>
diff --git a/pkg/compiler/lib/src/serialization/abstract_sink.dart b/pkg/compiler/lib/src/serialization/abstract_sink.dart
@@ -31,6 +31,7 @@ abstract class AbstractDataSink extends DataSinkMixin implements DataSink {
   IndexedSink<Uri> _uriIndex;
   IndexedSink<ir.Member> _memberNodeIndex;
   IndexedSink<ImportEntity> _importIndex;
+  IndexedSink<ConstantValue> _constantIndex;
 
   Map<Type, IndexedSink> _generalCaches = {};
 
@@ -49,6 +50,7 @@ abstract class AbstractDataSink extends DataSinkMixin implements DataSink {
     _uriIndex = new IndexedSink<Uri>(this);
     _memberNodeIndex = new IndexedSink<ir.Member>(this);
     _importIndex = new IndexedSink<ImportEntity>(this);
+    _constantIndex = new IndexedSink<ConstantValue>(this);
   }
 
   @override
@@ -465,6 +467,10 @@ abstract class AbstractDataSink extends DataSinkMixin implements DataSink {
   }
 
   void _writeConstant(ConstantValue value) {
+    _constantIndex.write(value, _writeConstantInternal);
+  }
+
+  void _writeConstantInternal(ConstantValue value) {
     _writeEnumInternal(value.kind);
     switch (value.kind) {
       case ConstantValueKind.BOOL:
diff --git a/pkg/compiler/lib/src/serialization/abstract_source.dart b/pkg/compiler/lib/src/serialization/abstract_source.dart
@@ -19,6 +19,7 @@ abstract class AbstractDataSource extends DataSourceMixin
   IndexedSource<Uri> _uriIndex;
   IndexedSource<_MemberData> _memberNodeIndex;
   IndexedSource<ImportEntity> _importIndex;
+  IndexedSource<ConstantValue> _constantIndex;
 
   Map<Type, IndexedSource> _generalCaches = {};
 
@@ -30,6 +31,7 @@ abstract class AbstractDataSource extends DataSourceMixin
     _uriIndex = new IndexedSource<Uri>(this);
     _memberNodeIndex = new IndexedSource<_MemberData>(this);
     _importIndex = new IndexedSource<ImportEntity>(this);
+    _constantIndex = new IndexedSource<ConstantValue>(this);
   }
 
   @override
@@ -533,6 +535,10 @@ abstract class AbstractDataSource extends DataSourceMixin
   }
 
   ConstantValue _readConstant() {
+    return _constantIndex.read(_readConstantInternal);
+  }
+
+  ConstantValue _readConstantInternal() {
     ConstantValueKind kind = _readEnumInternal(ConstantValueKind.values);
     switch (kind) {
       case ConstantValueKind.BOOL:
diff --git a/pkg/compiler/lib/src/serialization/helpers.dart b/pkg/compiler/lib/src/serialization/helpers.dart
@@ -335,8 +335,7 @@ class DartTypeNodeWriter
 /// Data sink helper that canonicalizes [E] values using indices.
 class IndexedSink<E> {
   final AbstractDataSink _sink;
-  final Map<E, int> _cache = {};
-  Set<E> _current = {};
+  final Map<E, int> _cache = {null: 0}; // slot 0 is pre-allocated to `null`.
 
   IndexedSink(this._sink);
 
@@ -345,16 +344,16 @@ class IndexedSink<E> {
   /// If [value] has not been canonicalized yet, [writeValue] is called to
   /// serialize the [value] itself.
   void write(E value, void writeValue(E value)) {
+    const int pending = -1;
     int index = _cache[value];
     if (index == null) {
-      if (!_current.add(value)) {
-        throw new ArgumentError("Cyclic dependency on cached value: $value");
-      }
       index = _cache.length;
       _sink._writeIntInternal(index);
+      _cache[value] = pending; // Increments length to allocate slot.
       writeValue(value);
       _cache[value] = index;
-      _current.remove(value);
+    } else if (index == pending) {
+      throw ArgumentError("Cyclic dependency on cached value: $value");
     } else {
       _sink._writeIntInternal(index);
     }
@@ -364,7 +363,7 @@ class IndexedSink<E> {
 /// Data source helper reads canonicalized [E] values through indices.
 class IndexedSource<E> {
   final AbstractDataSource _source;
-  final List<E> _cache = [];
+  final List<E> _cache = [null]; // slot 0 is pre-allocated to `null`.
 
   IndexedSource(this._source);
 
@@ -375,11 +374,17 @@ class IndexedSource<E> {
   E read(E readValue()) {
     int index = _source._readIntInternal();
     if (index >= _cache.length) {
+      assert(index == _cache.length);
+      _cache.add(null); // placeholder.
       E value = readValue();
-      _cache.add(value);
+      _cache[index] = value;
       return value;
     } else {
-      return _cache[index];
+      E value = _cache[index];
+      if (value == null && index != 0) {
+        throw StateError('Unfilled index $index of $E');
+      }
+      return value;
     }
   }
 }
