Blocked on removing debug mode

[leonsenft]

See #952 for context.

Today we conditionally generate debug code based on the compilation mode: debug or release. For example, a simple check for changes in release mode

if (!identical(_expr_0, currVal_0) {

is instead generated as

if (checkBinding(_expr_0, currVal_0)) {

in debug mode. This is not an exhaustive list of differences, it's merely one example.

The intent is to remove the debug mode altogether, and instead generate code that relies on dart2js optimizations. For example, we make use of

https://github.com/dart-lang/angular/blob/1ba3746c60597b92aebfb6edbadcba9754f04ddb/angular/lib/src/runtime/optimizations.dart#L23

to guard debug code. The Angular compiler will emit this code unconditionally, and rely on constant-propagation and dead-code elimination in dart2js to optimize away statements guarded by isDevMode (which to dart2js with assertions disabled, is false throughout the lifetime of the program).

The goal is to avoid regressing the code generated for release mode. With the new single compilation mode using isDevMode, we've managed to retain identical dart2js output in all but one place: the original example above. We now always emit

if (checkBinding(_expr_0, currVal_0)) {

in place of

if (!identical(_expr_0, currVal_0) {

and rely on isDevMode internally to call !identical() for release mode. Initially we noticed checkBinding() is too long to inline, which we fixed with @dart2js.tryInline. We confirmed this is due to dart2js performing inlining before dead-code elimination.

This led to the issue that is unresolved today. The optimized checkBinding() call yields slightly more JavaScript than !identical().

<         if (this._feed_component_template$_expr_0 !== currVal_0) {
---
>         t1 = this._feed_component_template$_expr_0;
>         if (t1 !== currVal_0) {

This is again likely due to the order in which dart2js performs optimizations; at an earlier phase before code was eliminated it probably made sense to perform the assignment to t1, but in the final result it seems pointless (t1 is immediately reassigned to something else inside the if-block).

Until we resolve this regression (or accept it), we won't be able to remove the distinct compilation modes.

[leonsenft]

Note this doesn't block the deprecation and removal of features in #952.

[matanlurey]

/cc @vsmenon @rakudrama @kevmoo - we need triage here.

This is currently the last issue blocking removing a large, ~unused feature from AngularDart.

[rakudrama]

The use of 'strange' temporaries in dart2js output is due to there being a 'ghost' of removed code in the compiler's representation. There is a removed if-then-else between the two lines of JavaScript.

It is not easy to remove the 'ghost' without disturbing JavaScript generation. We might end up by introducing temporaries in other places, for example, by failing to recognize opportunities to change other if-then-else regions into && and ||.

I would dearly love to rewrite this part of dart2js but a rewrite is several months of work.
May I suggest that we measure the size impact and find a saving of that size somewhere else?

[matanlurey]

@rakudrama:

May I suggest that we measure the size impact and find a saving of that size somewhere else?

I would also be fine with that!

We're honestly probably fine with the ghost variable, but we want to bring it up first before. We would also be happy with any sort of suggestions around how to structure our emitted code to avoid it (or some custom annotation in meta/dart2js.dart).

[rakudrama]

I don't think there is anything you can do here, other than give me a size target that unblocks you.
You could also take it on faith that we do care about size and will make some improvements after Dart 2 is shipped.

[matanlurey]

@rakudrama:

Definitely trust you, I'm just trying to make a good case for the size increase to my team.

[matanlurey]

We've decided to move forward accepting the regression. Thanks all!