dart-lang / sdk

The Dart SDK, including the VM, JS and Wasm compilers, analysis, core libraries, and more.
https://dart.dev
BSD 3-Clause "New" or "Revised" License
10.09k stars 1.56k forks source link

dart2js: Better code for ??= #35955

Open rakudrama opened 5 years ago

rakudrama commented 5 years ago

The generated code for a typical lazy-initialized field ...

  Data _data;
  Data get data => _data ??= _compute();

... is a bit verbose:

    get$data: function() {
      var t1 = this._data;
      if (t1 == null) {
        t1 = this._compute$0();
        this._data = t1;
      }
      return t1;
    }

What would it take to compile this to the following?

    get$data: function() {
      return this._data || (this._data = this.compute$());
    }
    get$data: function() {
      var t1 = this._data;
      if (t1 == null)
        t1 = this._data = this._compute$0();
      return t1;
    }

At this point SsaConditionMerger should recognize that a conditional (?:) can generated

    get$data: function() {
      var t1 = this._data;
      return t1 == null ? this._data = this._compute$0() : t1;
    },
    get$data: function() {
      var t1 = this._data;
      return !t1 ? this._data = this._compute$0() : t1;
    },
rakudrama commented 5 years ago

Regarding general field assignment chaining, the middle one is clearly an improvement. Is the last one a bridge too far?

        last = this._last;
        cell._previous = last;
        last._next = cell;
        this._last = cell;
 -->
        last = this._last;
        cell._previous = last;
        this._last = last._next = cell;
-->
        this._last = (cell._previous = this._last)._next = cell;
lrhn commented 5 years ago

If you worry about !t1 being slower than t1 == null, consider swapping the branches and just do t1 ? t1 : this._data = this._computer$0(). (That is, if the test of a two-branch condition is a negation, remove the negation and swap the branches. That should be safe since ! use the same falsy-logic as the condition).

That migth also be easier for a JS-level optimizier to recongize as equivalent to t1 || this._data = this._compute$0().

rakudrama commented 5 years ago

@lrhn

A JS-level peephole optimizer would work, but since dart2js does variable allocation ahead of generating JavaScript, doing so would leave a temporary that is assigned once and used once, i.e.

var t1 = this._data;
return t1 || (this._data = this._compute$());

which is quite a bit larger than

return this._data || (this._data = this._compute$());

I'm not worried about !t1 ? ... vs t1 ? ..., they are pretty much the same - both incur a full ToBoolean conversion, which has 9 tests to cover the full glory of JavaScript truthiness. What we are giving up is that t1 == null can be done with 3 tests (null, undefined, and undetectable objects) vs the full 9 tests.