[dart2js] H.JsLinkedHashMap.prototype missing $index method

[dcharkes]

When running the new MapLookup benchmark in dart2js it crashes.

$ out/ReleaseX64/dart-sdk/bin/dart2js --packages=.packages --out=out.js benchmarks/MapLookup/dart/MapLookup.dart >&2 && third_party/d8/linux/x64/d8 --stack_size=1024 sdk/lib/_internal/js_runtime/lib/preambles/d8.js out.js
Compiled 8,945,317 characters Dart to 114,794 characters JavaScript in 0.67 seconds
Dart file benchmarks/MapLookup/dart/MapLookup.dart compiled to JavaScript: out.js
MapLookup.Constant1(RunTimeRaw): 28.109627547434997 ns.
MapLookup.Constant5(RunTimeRaw): 16.74410816693876 ns.
sdk/lib/_internal/js_runtime/lib/preambles/d8.js:263: TypeError: map.$index is not a function
          throw e;
          ^
TypeError: map.$index is not a function
    at Final1.measureFor$1 (out.js:2954:21)
    at main (out.js:256:12)
    at action (sdk/lib/_internal/js_runtime/lib/preambles/d8.js:275:31)
    at eventLoop (sdk/lib/_internal/js_runtime/lib/preambles/d8.js:258:9)
    at self.dartMainRunner (sdk/lib/_internal/js_runtime/lib/preambles/d8.js:276:5)
    at out.js:3114:7
    at out.js:3099:7
    at dartProgram (out.js:3110:5)
    at out.js:3118:3

The crash occurs with the benchmark file as is, but also with some benchmarks cut out.

  final benchmarks = [
    () => const Constant1(),
    () => const Constant5(),
    () => const Final1(), // This crashes
  ];

However, removing one of the earlier benchmarks makes the next one succeed.

  final benchmarks = [
    () => const Constant5(),
    () => const Final1(), // This succeeds now
  ];

The diff in out.js indeed shows the missing $index method in H.JsLinkedHashMap.prototype:

253,254c253,254
<         benchmarks = [new G.main_closure(), new G.main_closure0()];
<       for (_i = 0; _i < 2; ++_i) {
---
>         benchmarks = [new G.main_closure(), new G.main_closure0(), new G.main_closure1()];
>       for (_i = 0; _i < 3; ++_i) {
262a263,265
>     Constant1: function Constant1(t0) {
>       this.name = t0;
>     },
271a275,276
>     },
>     main_closure1: function main_closure1() {
2671,2700d2675
<     $index: function(_, key) {
<       var strings, cell, t1, nums, _this = this, _null = null;
<       if (typeof key == "string") {
<         strings = _this._strings;
<         if (strings == null)
<           return _null;
<         cell = _this._getTableCell$2(strings, key);
<         t1 = cell == null ? _null : cell.hashMapCellValue;
<         return t1;
<       } else if (typeof key == "number" && (key & 0x3ffffff) === key) {
<         nums = _this._nums;
<         if (nums == null)
<           return _null;
<         cell = _this._getTableCell$2(nums, key);
<         t1 = cell == null ? _null : cell.hashMapCellValue;
<         return t1;
<       } else
<         return _this.internalGet$1(key);
<     },
<     internalGet$1: function(key) {
<       var bucket, index,
<         rest = this.__js_helper$_rest;
<       if (rest == null)
<         return null;
<       bucket = this._getTableBucket$2(rest, J.get$hashCode$(key) & 0x3ffffff);
<       index = this.internalFindBucketIndex$2(bucket, key);
<       if (index < 0)
<         return null;
<       return bucket[index].hashMapCellValue;
<     },
2985a2961,2965
>   G.Constant1.prototype = {
>     get$myMap: function() {
>       return C.Map_9MkqK;
>     }
>   };
2998c2978
<       return C.Constant5_mKQ;
---
>       return C.Constant1_yF4;
3004c2984
<       return C.Final1_3Fy;
---
>       return C.Constant5_mKQ;
3007a2988,2993
>   G.main_closure1.prototype = {
>     call$0: function() {
>       return C.Final1_3Fy;
>     },
>     $signature: 5
>   };
3022c3008
<     _inheritMany(H.Closure, [H.Primitives_initTicker_closure, H.TearOffClosure, P.MapBase_mapToString_closure, P.Duration_toString_sixDigits, P.Duration_toString_twoDigits, G.main_closure, G.main_closure0]);
---
>     _inheritMany(H.Closure, [H.Primitives_initTicker_closure, H.TearOffClosure, P.MapBase_mapToString_closure, P.Duration_toString_sixDigits, P.Duration_toString_twoDigits, G.main_closure, G.main_closure0, G.main_closure1]);
3028c3014
<     _inheritMany(G.MapLookupBenchmark, [G.Final1, G.Constant5]);
---
>     _inheritMany(G.MapLookupBenchmark, [G.Constant1, G.Final1, G.Constant5]);
3034c3020
<     types: ["int()", "String(int)", "~(Object?,Object?)", "Constant5()", "Final1()"],
---
>     types: ["int()", "String(int)", "~(Object?,Object?)", "Constant1()", "Constant5()", "Final1()"],
3037c3023
<   H._Universe_addRules(init.typeUniverse, JSON.parse('{"JSBool":{"bool":[]},"JSArray":{"List":["1"],"Iterable":["1"]},"JSUnmodifiableArray":{"JSArray":["1"],"List":["1"],"Iterable":["1"]},"JSNumber":{"num":[]},"JSInt":{"int":[],"num":[]},"JSNumNotInt":{"num":[]},"JSString":{"String":[]},"ConstantStringMap":{"ConstantMap":["1","2"]},"JsLinkedHashMap":{"MapMixin":["1","2"],"LinkedHashMap":["1","2"]},"MapBase":{"MapMixin":["1","2"]},"int":{"num":[]},"Final1":{"MapLookupBenchmark":[]},"Constant5":{"MapLookupBenchmark":[]}}'));
---
>   H._Universe_addRules(init.typeUniverse, JSON.parse('{"JSBool":{"bool":[]},"JSArray":{"List":["1"],"Iterable":["1"]},"JSUnmodifiableArray":{"JSArray":["1"],"List":["1"],"Iterable":["1"]},"JSNumber":{"num":[]},"JSInt":{"int":[],"num":[]},"JSNumNotInt":{"num":[]},"JSString":{"String":[]},"ConstantStringMap":{"ConstantMap":["1","2"]},"JsLinkedHashMap":{"MapMixin":["1","2"],"LinkedHashMap":["1","2"]},"MapBase":{"MapMixin":["1","2"]},"int":{"num":[]},"Constant1":{"MapLookupBenchmark":[]},"Final1":{"MapLookupBenchmark":[]},"Constant5":{"MapLookupBenchmark":[]}}'));
3041a3028
>       ConstantStringMap_String_String: findType("ConstantStringMap<String,String>"),
3070a3058
>     C.Constant1_yF4 = new G.Constant1("MapLookup.Constant1");
3074a3063,3064
>     C.List_9Mi = H._setArrayType(makeConstList(["0"]), type$.JSArray_String);
>     C.Map_9MkqK = new H.ConstantStringMap(1, {"0": "1"}, C.List_9Mi, type$.ConstantStringMap_String_String);
3076c3066
<     C.Map_MYyTv = new H.ConstantStringMap(5, {"0": "1", "1": "2", "2": "3", "3": "4", "4": "5"}, C.List_MYA, H.findType("ConstantStringMap<String,String>"));
---
>     C.Map_MYyTv = new H.ConstantStringMap(5, {"0": "1", "1": "2", "2": "3", "3": "4", "4": "5"}, C.List_MYA, type$.ConstantStringMap_String_String);

cc @sigmundch @rakudrama

[joshualitt]

I thought this could be related to changes made to support deferred holders. However, this bug seems to be much older. I still see this issue even after reverting until August 17th, 2020.

[sortie]

Hi, do we have any ideas for this issue? Otherwise we should disable these benchmarks on dart2js to save the resources.

[rakudrama]

I will take a look.

Question about the benchmark - why is it showing in the charts as RunTimeRaw rather than Runtime as Score like the majority of benchmarks? Perhaps this could be selectable? I would always use Score for the left axis of chart, but would like to be able to see the raw numbers, perhaps as a right-axis on the large charts, or in the hover info.

Using Score has the advantage that each 2x performance increase looks similar and gives a readable chart for ongoing improvements, whereas a raw time on the axis has the effect of making each 2x speed increase look less and less significant. A temporary 100x regression on a chart with a 'raw' axis yields a chart that is unusable.

[sortie]

Thanks Stephen :)

The VM team often care about the absolute time to do a particular operation and tends to prefer RunTimeRaw for that reason, if I understood correctly. The benchmark author can choose what they prefer. I agree about the usability differences and different mind sets and I want to unify RunTime and RunTimeRaw more in the future as it is just a different perspective on the same data. I believe we already have a feature enhancement filed about this topic.

[whesse]

I didn't see this issue, investigated the same problem, and came up with a reduced test case for this. I also can get a slightly different error if I pass the --benchmarking-production flag to dart2js. I get a "map.get$length is not a function" Without the --benchmarking-production flag, I get a "t1.toString$0 is not a function" on the output of the previous map.get$length.

The reduced code, compiled with

and run with

third_party/d8/linux/x64/d8  sdk/lib/_internal/js_runtime/lib/preambles/d8.js out.jsis 

is

abstract class MapLookupBenchmark {
   MapLookupBenchmark();

  Map<String, String> get myMap;

 void printLength() {
     final map = myMap;
   print(map.length);
  }
}

const const1 = <String, String>{
  '0': '1',
};

final final1 = <String, String>{
  '0': '1',
};

const const5 = <String, String>{
  '0': '1',
  '1': '2',
  '2': '3',
  '3': '4',
  '4': '5',
};

class Constant1 extends MapLookupBenchmark {
  Constant1();

  @override
  Map<String, String> get myMap => const1;
}

class Final1 extends MapLookupBenchmark {
   Final1();

  @override
  Map<String, String> get myMap => final1;
}

class Constant5 extends MapLookupBenchmark {
  Constant5();

  @override
  Map<String, String> get myMap => const5;
}

void main() {
  Constant1();
  Constant5();
  // print(Final1().myMap.length); // No failure if this is uncommented.
  Final1().printLength();
}