Sdk: dart2js: 10x worse performance in lzma than DDC

[PixelToast]

• Dart version: Dart VM version: 2.3.0 (Fri May 3 10:32:31 2019 +0200) on "windows_x64"
• Chrome version: 74.0.3729.131 (Official Build) (64-bit) (cohort: Stable)
• Firefox version: 66.0.3 (64-bit)

main.dart:

import 'dart:html';
import 'package:lzma/lzma.dart';
void main() async {
  await Future.delayed(Duration(seconds: 1));
  var t0 = DateTime.now();
  for (int i = 0; i < 10; i++) {
    lzma.encode((("ASDF$i" * 10).codeUnits));
  }
  var t1 = DateTime.now();
  document.body.text = "dt: ${t1.millisecondsSinceEpoch - t0.millisecondsSinceEpoch}";
}

Benchmark results:
webdev serve
Chrome: 3,683ms
Firefox: 10,969ms

webdev serve --release
Chrome: 43,095ms
Firefox: 11,536ms

[kevmoo]

CC @rakudrama @sigmundch

[sigmundch]

Could you provide what the results look like if you add -O3 or -O4 as a flag to the dart2js compiler?

By default to comply with the Dart2 semantics we add many extra checks that today we don't optimize away (yet). The checks can be expensive, so until we add the extra optimizations we added the -O3 and -O4 flags to remove the extra checks entirely. Many customers use these flags when deploying logic that is performance sensitive, and we basically recommend doing so under the assumption that the code was tested with the checks extensively.

[jodinathan]

shouldn't webdev --release use -03 or -04 by default?
I always thought it was the default behavior.

[PixelToast]

webdev didn't appear to be passing -O3 or -O4 by default:
[INFO] build_web_compilers:entrypoint on web/main.dart: Running dart2js with --minify --packages=.package-eb69edfde2318f0ace563aeb9e18ce74 -oweb/main.dart.js web/main.dart

Adding

targets:
  $default:
    builders:
      build_web_compilers|entrypoint:
        options:
          compiler: dart2js
          dart2js_args:
            - -O4

To my build.yaml passes -O4 as expected

(Chrome)
-O4: 1,936ms
-O3: 2,115ms
-O2: 46,172ms

Problem appears to be solved

[jakemac53]

shouldn't webdev --release use -03 or -04 by default?

We don't do that today since they are technically not safe for all programs. In practice I have never seen issues though so we might consider changing it.

[kevmoo]

Can close! @vsmenon – we may want to talk about giving better guidance here...

Even a note from build_web_compilers?

[matanlurey]

@kevmoo @vsmenon Even with -O2 should Dart2JS be 10x slower than DDC?

It is probably worth keeping this open so @rakudrama or others can investigate what code path makes an optimizing compiler 10x slower than a dev one?

[kevmoo]

@matanlurey ack – will let @rakudrama et el handle

[sigmundch]

We can take a closer look, but I wouldn't be surprised if the problem is basically that a couple methods that are hot have extra unnecessary type checks that one day we'll be able to optimize away.

In terms of the default, it's a good question. The -O3 flag and up make assumptions that your program has no errors thrown at runtime (exceptions can be thrown, but errors not). To ensure that the optimizations work properly, we expect that users only enable it if their code was well tested.

If we default to -O3, I'm worried that users will not be as aware of this assumption. @vsmenon proposed a middle-ground: could be to surface to the webdev tool an additional flag to make this more explicit? @jakemac53 what do you think? Say webdev --release --O3?

[jodinathan]

I guess that webdev --release should assume that the code is well tested.
So maybe there should be an option like "webdev --homolog" that do what webdev --release does today.
Maybe a new flag for --03 like webdev --production.

[natebosch]

We only have 2 flavors of build today dev and release. Adding a third flavor would be quite invasive and we'd have to rethink the flags.

We could plausibly give a UX like webdev build --O3 with https://github.com/dart-lang/build/issues/1544

The downside there is we currently only support _overwriting_ dart2js_flags, not appending or replacing.

So in addition to configuration set aliases I believe we need to add specific --O level flag support to build_web_compilers:entrypoint and handle smartly merging with dart2js_flags.

[matanlurey]

I would be more interested in knowing what customers want -O2 by default. I can't think of any.

[vsmenon]

@rakudrama @sigmundch @kevmoo

We should probably break this out into two issues:

• Type checks in dart2js are slow. (I think @rakudrama 's type rep work may address some of this?)
• Make > -O3 / unsafe optimizations more discoverable (or possibly default) in webdev and other tools.

[sigmundch]

Sounds good to track pieces separately.

Type checks in dart2js are slow.

Besides the new RTI (see #36085) we also need to add entrypoint specialization that allows us to elide checks when we can statically tell that they are not needed (see #34003).

Make > -O3 / unsafe optimizations more discoverable (or possibly default) in webdev and other tools.

Filed https://github.com/dart-lang/build/issues/2279 to track this.

@matanlurey - let us know if the 3 issues above are tracking everything we need to track, if so, we can close this current issue.

[vsmenon]

Closed in favor of above issues.