Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Multiple-comparison optimization suggestion #16597

Open
stevemessick opened this issue Feb 6, 2014 · 4 comments
Open

Multiple-comparison optimization suggestion #16597

stevemessick opened this issue Feb 6, 2014 · 4 comments
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. dart2js-optimization type-enhancement A request for a change that isn't a bug web-dart2js

Comments

@stevemessick
Copy link
Contributor

[user feedback]

Dart2Js generates the following code for me:

removeEntity$1: [function(entity) {
      var t1 = J.getInterceptor(entity);
      if (typeof entity === "object" && entity !== null && !!t1.$isAmbientLightEntity)
        C.JSArray_methods.remove$1(this._ambientLights, entity);
      if (typeof entity === "object" && entity !== null && !!t1.$isDirectionalLightEntity)
        C.JSArray_methods.remove$1(this._directionalLights, entity);
      if (typeof entity === "object" && entity !== null && !!t1.$isPointLightEntity)
        C.JSArray_methods.remove$1(this._pointLights, entity);
      if (typeof entity === "object" && entity !== null && !!t1.$isSpotLightEntity)
        C.JSArray_methods.remove$1(this._spotLights, entity);
      E.RendererComponent.prototype.removeEntity$1.call(this, entity);
    }, "call$1", "get$removeEntity", 2, 0, null, 676],

based on:

  void removeEntity(Entity entity) {
    if (entity is AmbientLightEntity) {
      _ambientLights.remove(entity);
    }

    if (entity is DirectionalLightEntity) {
      _directionalLights.remove(entity);
    }

    if (entity is PointLightEntity) {
      _pointLights.remove(entity);
    }

    if (entity is SpotLightEntity) {
      _spotLights.remove(entity);
    }

    super.removeEntity(entity);
  }

Dart2Js should notice that it is checking the same thing multiple times (entity == null) and move it into an outer if statement.
////////////////////////////////////////////////////////////////////////////////////
Editor: 1.2.0.dev_02_04 (2014-01-31)
OS: Windows 8 - amd64 (6.2)
JVM: 1.7.0_21

projects: 1

open dart files: 0

auto-run pub: true
localhost resolves to: 127.0.0.1
mem max/total/free: 1778 / 784 / 113 MB
thread count: 29
index: 559247 relationships in 108843 keys in 15293 sources

SDK installed: true
Dartium installed: true

@floitschG
Copy link
Contributor

Added Optimization label.

@rakudrama
Copy link
Member

True. We don't have an optimization for

if (a && b) ...
if (a && c) ...

--->
if (a) {
  if (b) ...
  if (c) ...
}

In fact we have no optimizations the change the shape of the SSA graph.

In this case we could make a different improvement. The canned check

  typeof entity === "object" && entity !== null && !!t1.$isXXX

could be replaced by

  t1.$isXXX

since it is reading from an interceptor.

There are some cases where we don't use the interceptor.
To enable the control flow optimization, we would have to introduce the null/primitive dereference check as a separate instruction.

@rakudrama
Copy link
Member

This reduces the code size by always using the interceptor https://codereview.chromium.org/188433002/

@floitschG
Copy link
Contributor

Removed the owner.

@kevmoo kevmoo added type-enhancement A request for a change that isn't a bug and removed priority-unassigned labels Feb 29, 2016
@matanlurey matanlurey added the closed-obsolete Closed as the reported issue is no longer relevant label Jun 19, 2018
@rakudrama rakudrama reopened this Jun 19, 2018
@rakudrama rakudrama removed the closed-obsolete Closed as the reported issue is no longer relevant label Jun 19, 2018
@vsmenon vsmenon added the area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. label Jul 20, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-web Use area-web for Dart web related issues, including the DDC and dart2js compilers and JS interop. dart2js-optimization type-enhancement A request for a change that isn't a bug web-dart2js
Projects
None yet
Development

No branches or pull requests

6 participants