dart2js - minor optimization opportunity for identical check

[DartBot]

This issue was originally filed by @bp74

---

What steps will reproduce the problem?

I have a very hot function and tried to optimize it for dart2js. Thereby i noticed an optimization opportunity which doesn't improve the performance but still it would be a nice if the compiler knows how to optimize it.

Dart code (first version):

  if (foo != null && !identical(foo, _foo)) {
    // Do stuff
  }

Dart code (second version):

  if (foo!= null) {
    if (!identical(foo, _foo)) {
      // Do stuff
    }
  }

What is the expected output? What do you see instead?

The first version generates this JavaScript code:

  var t1;
  if (foo != null) {
    t1 = this._foo;
    t1 = foo == null ? t1 != null : foo !== t1;
  } else
    t1 = false;
  if (t1) {
    // Do stuff
  }

The second version generates this JavaScript code:

  var t1;
  if (foo != null) {
    t1 = this._foo;
    if (foo == null ? t1 != null : foo !== t1) {
      // Do stuff
    }
  }

I can imagine that the first version of the Dart code may be harder to optimize, but the second version should be easier because the test for "foo == null" is always false in this block. In a perfect world the compiler would recognize this in the first version too and the JavaScript code would look like this:

  if (foo != null && foo !== _foo) {
    // Do stuff
  }

What version of the product are you using? On what operating system?

Dart Editor version 1.2.0.dev_01_00 (DEV)
Dart SDK version 1.2.0-dev.1.0

Please provide any additional information below.

[sgjesse]

Removed Type-Defect label.
Added Type-Enhancement, Area-Dart2JS, Triaged labels.

[rakudrama]

Bernhard, I tried to reproduce this problem, but my simple examples generate the optimized code your are expecting.

Can you provide us with a repro case where the optimization is missing?

[DartBot]

This comment was originally written by @bp74

---

Thanks for trying, i will look at my code and hopefully i can provide a simple test case.

[DartBot]

This comment was originally written by @bp74

---

Okay, here is a test case. I tried it with the stable and dev channel:

Dart Editor version 1.1.1.release (STABLE)
Dart Editor version 1.2.0.dev_01_00

==============================
Dart code:

class foo {
  String _a;
  String _b;
  update(String a, String b) {
    if (a != null && !identical(a, _a)) {
      _a = a;
    }
    if (b != null) {
      if (!identical(b, _b)) _b = b;
    }
  }
}

void main() {
  var f = new foo();
  f.update(null, "b");
  f.update("a", null);
  f.update("a", "b");
}

==============================
JavaScript code:

foo: {
    "": "Object;_a,_b",
    update$2: function(a, b) {
      var t1;
      if (a != null) {
        t1 = this._a;
        t1 = a == null ? t1 != null : a !== t1;
      } else
        t1 = false;
      if (t1)
        this._a = a;
      if (b != null) {
        t1 = this._b;
        if (b == null ? t1 != null : b !== t1)
          this._b = b;
      }
    }
  }

[floitschG]

Thanks.

---

cc @herhut-ggl.
Added Accepted label.

[rakudrama]

I had a look at this. In practice I saw only a few improvements - there are only a handful in swarm. The few improvements are mostly removal of a receiver check or argument check when the value is explicitly checked against null.

https://codereview.chromium.org/136763012 is what I did.

Considering that it requires tracking types per-use rather than per-def, I think it better to leave it for the new IR.

If codegen didn't require the removal of HTypeKnown nodes, types could be tracked per-def.

Kevin: you are cc-ed because you might be interested in the issue of tracking types per-use vs per-def.

---

cc @kmillikin.

[floitschG]

Added Optimization label.