[js_interop/dart2js] Constant fold string literals `.toJS` in dart2js

[mkustermann]

The following example

import 'dart:js_interop';
import 'dart:js_interop_unsafe';

@JS()
external JSObject eval(JSString expression);

JSString lookupKey(JSObject jsObject) => jsObject.getProperty(kKey);

final JSString kKey = 'key'.toJS;

main() {
  final jsMap = eval('{key: "value"}'.toJS);
  print(lookupKey(jsMap).toDart);
}

compiles (with dart compile js --no-minify -O4 -o repro.js repro.dart) to:

...
  (function lazyInitializers() {
    var _lazyFinal = hunkHelpers.lazyFinal;
    ...
    _lazyFinal($, "kKey", "$get$kKey", () => "key");
  })();
...
  main() {
    A.printString(self.eval('{key: "value"}')[$.$get$kKey()]);
  }
...

When instead manually inlining this in dart to

import 'dart:js_interop';
import 'dart:js_interop_unsafe';

@JS()
external JSObject eval(JSString expression);

JSString lookupKey(JSObject jsObject) => jsObject.getProperty('key'.toJS);

main() {
  final jsMap = eval('{key: "value"}'.toJS);
  print(lookupKey(jsMap).toDart);
}

then this becomes

    main() {
      A.printString(self.eval('{key: "value"}').key);
    }

/cc @srujzs @rakudrama

[dart-github-bot]

Labels: area-web, type-bug Summary: The toJS method on string literals is not being constant folded in dart2js, resulting in unnecessary JavaScript code generation. This leads to larger and less efficient code compared to manually inlining the string literal.

[rakudrama]

Constant-folding of final top-level/static variables is mostly done by CFE. final foo = 123; is handled somewhat like const foo = 123; because dart2js can see the RHS is a constant in the Kernel IR. Unfortunately, dart2js does not compile using a full callgraph-bottom-up schedule, so the non-const initializer expression that reduces to a constant it not seen by the users of the variable.

We could do something more limited if this is a popular code pattern./

[mkustermann]

We could do something more limited if this is a popular code pattern./

The reason I used that pattern in a benchmark is because on dart2wasm this is not a NOP and we'd do the key conversion every single time we do a map lookup. So it makes sense to hoist it there to top-level which then regresses dart2js.

If we recognize this specially in dart2wasm compiler (see https://github.com/dart-lang/sdk/issues/56046) then using the pattern inline in code (rather than top-levels) would not be needed.

Though any 3rd party user may also think it's more efficient to cache in a top level instead of executing every time - and it would be worse performance. So I think maybe it's a good idea to do this optimization regardless.