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

Remove leaf operations which have no side-effects #3252

Closed
mhevery opened this issue Jul 23, 2015 · 6 comments
Closed

Remove leaf operations which have no side-effects #3252

mhevery opened this issue Jul 23, 2015 · 6 comments

Comments

@mhevery
Copy link
Contributor

mhevery commented Jul 23, 2015

There are a lot of statements in code which only make sense if there is a next item following. If it is a leaf they should be removed.

Example:

    var change_interpolate1 = false; // NOT NEEDED


    if (change_componentName0) {
      currentProto = _protos[1];
      interpolate1 = "" "${componentName0 == null ? "" : componentName0}" "";
      if (!_gen.looseIdentical(interpolate1, _interpolate1)) {
        change_interpolate1 = true;  // NOT NEEDED
        if (throwOnChange) {
          _gen.ChangeDetectionUtil.throwOnChange(_protos[1],
              _gen.ChangeDetectionUtil.simpleChange(
                  _interpolate1, interpolate1));
        }

        _dispatcher.notifyOnBinding(currentProto.bindingRecord, _interpolate1 = interpolate1);

        _interpolate1 = interpolate1;
      }
    } else {  // NOT NEEDED
      interpolate1 = _interpolate1; // NOT NEEDED
    }

Notice the else and assignment at the end. This is only needed if there would be a next item dereferencing it..

should be:

    if (change_componentName0) {
      currentProto = _protos[1];
      interpolate1 = "" "${componentName0 == null ? "" : componentName0}" "";
      if (!_gen.looseIdentical(interpolate1, _interpolate1)) {
        if (throwOnChange) {
          _gen.ChangeDetectionUtil.throwOnChange(_protos[1],
              _gen.ChangeDetectionUtil.simpleChange(
                  _interpolate1, interpolate1));
        }

        _dispatcher.notifyOnBinding(currentProto.bindingRecord, _interpolate1 = interpolate1);

        _interpolate1 = interpolate1;
      }
    }

This requires some form of detecting leaves and stripping out the vars which are used of chaining.

@mhevery mhevery added comp: core/change_detection effort2: days refactoring Issue that involves refactoring or code-cleanup labels Jul 23, 2015
@kegluneq
Copy link

@jakemac53 please evaluate and determine if this gives any benefit after dart2js compilation.

@jakemac53
Copy link
Contributor

Definitely the change_* booleans get filtered out by dart2js if the value is never used.

@yjbanov yjbanov added area: performance and removed refactoring Issue that involves refactoring or code-cleanup labels Jul 28, 2015
@jakemac53
Copy link
Contributor

@yjbanov is this still relevant based on the changes coming?

@vsavkin
Copy link
Contributor

vsavkin commented Jul 29, 2015

Sorry, I fixed it in #3361 by accident. I am reassigning this to myself.

@vsavkin vsavkin assigned vsavkin and unassigned jakemac53 Jul 29, 2015
@vsavkin vsavkin closed this as completed Jul 29, 2015
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants