Prohibitively high cost of "late" fields and variables

[matanlurey]

First off, I totally recognize late right now is being (in most cases?) directly lowered by the CFE, and not optimized 👍 . I'd probably still recommend using late where it makes the code significantly cleaner, but otherwise I don't quite understand why it's not just trading an NPE on (!.) for an equally cryptic LateInitializeErrorImpl and a lot of runtime + download cost.

I evaluated using late in two places today when migrating code, and decided against it in both cases:

Instance-level final field

-// @dart=2.9
+
class NgZone {
  final _outerZone = Zone.current;
- Zone _innerZone;
+ late final Zone _innerZone;
  NgZone() {
    _innerZone = _createInnerZone(_outerZone);
  }

  // ... Additional methods that access _innerZone ...
}

... and a null-check with an inlined error handler is emitted on every access point:

accessInnerZone$0: function() {
  var t1 = this.__NgZone_innerZone;
  if (t1 == null)
    t1 = H.throwExpression(H.LateInitializationErrorImpl$("Field '_innerZone' has not been initialized."));
  t1.useInstanceMethod$0();
}

Using a nullable type and a ! (and -o3) was much more terse:

-// @dart=2.9
+
class NgZone {
  final _outerZone = Zone.current;
- Zone _innerZone;
+ Zone? _innerZone;
  NgZone() {
    _innerZone = _createInnerZone(_outerZone);
  }

  // ... Additional methods that access _innerZone ...
}

accessInnerZone$0: function() {
  this.__NgZone_innerZone.useInstanceMethod$0();
}

Local variable

void funcWithNullableLocal(List<Foo> queue) {
- Foo foo;
+ late final Foo foo;
  void onCallback() {
    queue.remove(foo);
  }
  foo = Foo(onCallback);
  queue.add(foo);
}

funcWithLateFinalLocal: function(queue) {
  var t2, t1 = {};
  t1.foo = null;
  t2 = new R.funcWithLateFinalLocal__foo_get(t1);
  new R.funcWithLateFinalLocal__foo_set(t1).call$1(new R.Foo(new R.funcWithLateFinalLocal_onCallback(queue, t2)));
  queue.push(t2.call$0());
},
R.funcWithLateFinalLocal__foo_set.prototype = {
  call$1: function(t1) {
    var t2 = this._box_0;
    if (t2.foo == null)
      return t2.foo = t1;
    else
      throw H.wrapException(H.LateInitializationErrorImpl$("Local 'foo' has already been initialized."));
  }
};
R.funcWithLateFinalLocal__foo_get.prototype = {
  call$0: function() {
    var t1 = this._box_0.foo;
    return t1 == null ? H.throwExpression(H.LateInitializationErrorImpl$("Local 'foo' has not been initialized.")) : t1;
  }
};
R.funcWithLateFinalLocal_onCallback.prototype = {
  call$0: function() {
    C.JSArray_methods.remove$1(this.queue, this._foo_get.call$0());
  }
};

... compared with nullable Foo:

void funcWithNullableLocal(List<Foo> queue) {
- Foo foo;
+ Foo? foo;
  void onCallback() {
    queue.remove(foo);
  }
  foo = Foo(onCallback);
  queue.add(foo);
}

funcWithNullableLocal: function(queue) {
  var foo, t1 = {};
  t1.foo = null;
  foo = new R.Foo(new R.funcWithNullableLocal_onCallback(t1, queue));
  t1.foo = foo;
  queue.push(foo);
},
funcWithNullableLocal_onCallback: function funcWithNullableLocal_onCallback(t0, t1) {
  this._box_0 = t0;
  this.queue = t1;
},

/cc @leafpetersen @rakudrama - I can connect with you internally this week to see how I can help further!

[matanlurey]

I was just made aware (thanks @jonahwilliams) that Dart2JS is entirely relying on CFE lowering as-per here.

[lrhn]

The instance variable is a hard case to optimize well. The _createInnerZone method leaks a lot of this pointers, so it's very hard to detect whether _innerZone is definitely initialized in any code. Adding ! everywhere will likely generate the same amount of checks, but possibly a smaller amount of code. Or we handle null-checks more efficiently (perhaps by just calling methods on the value and letting it throw, if it's in receiver position). This is where it hurts to specify which error the invalid access throws.

Should we just not do that, and simply say that "a dynamic error occurs if when reading an uninitialized late variable"?

For the local variable, you are saved by promotion. The queue.add(foo) knows that the foo is definitely assigned, and the code for late should be just as efficient when it gets properly implemented, and not just brute-force rewritten to an abstraction.

The queue.remove is not included in your output, so I can't see how it's compiled.
Here you are actually making a mistake: queue.remove(foo) is removing the nullable value, even if it's null. That's accidentally correct because the queue element type is non-nullable, so removing null is a (potentially expensive) no-op.
Still, it's not doing the same thing as the late code. You should at least add a ! to make it queue.remove(foo!), and then check what code it generates.

[rakudrama]

@lrhn Yes!
dart2js compiles ! to a property access. This can often be folded into the next property access and disappear, making the test free. We allowed the failure of ! to be a TypeError to permit that optimization. I'd like to be able to do the same with late variables with non-nullable types. We won't be able to tell at run time if the error is a TypeError from !, a NoSuchMethodError from a dynamic call with a null receiver, or a LateInitializationError, since they all will generate the same code, often zero extra code.

[leonsenft]

I found another code size issue with late final (with no initializer) that Matan's code snippets don't capture.

In some cases, dart2js coalesces multiple field initializations into a single statement.

// Before null-safety
abstract class HostView<T> {
  T component;
  ComponentView<T> componentView;
  Injector _injector;
}

// constructor initialization
_._host_view$_injector = _.componentView = _.component = null;

We've found this only occurs when fields with the same initial value are adjacent, and have made a concerted effort to ensure our class layout takes advantage of this optimization.

These fields were ideal candidates for late final, as they're initialized in one generated method before they're used in another. However, making these fields late final has a detrimental affect on field initialization:

// After null-safety
abstract class HostView<T> {
  late final T component;
  late final ComponentView<T> componentView;
  late final Injector _injector;
}

// constructor initialization
_.__HostView_component = null;
_.__HostView_component_isSet = false;
_.__HostView_componentView = null;
_.__HostView_componentView_isSet = false;
_.__HostView__injector = null;
_.__HostView__injector_isSet = false;

Ideally this could be:

_.__HostView_component = _.__HostView_componentView = _.__HostView__injector = null;
_.__HostView_component_isSet = _.__HostView_componentView_isSet = _.__HostView__injector_isSet = false;

[leafpetersen]

Should this slip to April or are we still trying to land something here?

[rakudrama]

Lets slip to April. We have some things we are trying to land, but the more consequential changes are not imminent.

[franklinyow]

@sigmundch status?

[sigmundch]

lowering changes landed and some improvements made it in, we still have however some optimizations to do and the option to elide initialization checks in -O3

[vsmenon]

@sigmundch - moving this to the September milestone. May want to consider -O3 check elision as a separate P1?

[sigmundch]

thanks, agree - I filed #46806 to track that separately, but it is all related and we have a few overlapping work items.

@fishythefish - feel free to reorganize the issues in a way that it's most helpful to you.

[rakudrama]

I am going to close out the issue since there have been improvements in all the reported examples

• The cost of a late check anywhere is similar to, and sometimes smaller than, the equivalent manually-written code
• There are optimizations for late fields which would not apply to the manual code