Skip to content
New issue

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

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

Already on GitHub? # to your account

Mangler performance - some improvement #134

Merged
merged 2 commits into from
Aug 31, 2016
Merged

Mangler performance - some improvement #134

merged 2 commits into from
Aug 31, 2016

Conversation

boopathi
Copy link
Member

This gives a ~=200ms of mangler perf.

  1. If NOT shouldConsiderSource, don't traverse Identifier & Literals for charset considerations
  2. Don't recalculate hasBinding
  3. loops

Most time was spent getting and verifying scope's bindings.

The following difference is between scope.hasBinding(next) and bindings.hop(next)

run-mangle                 740.549  1        740.549       
scopable-loop              547.912  2222     0.247         
run-collect                370.834  1        370.834       
next-identifier            222.018  3693     0.060         
get-all-bindings           171.469  2222     0.077         
Collect-Identifier         65.521   19176    0.003         
compute-oldname            54.560   64879    0.001         
scopable-rename            49.586   3693     0.013         
rename                     42.556   3693     0.012         

and now after this

run-mangle                 558.121  1        558.121       
scopable-loop              378.939  2222     0.171         
run-collect                369.423  1        369.423       
get-all-bindings           180.804  2222     0.081         
compute-oldname            53.859   64879    0.001         
scopable-rename            53.098   3693     0.014         
rename                     47.032   3693     0.013         
Collect-Identifier         44.661   19176    0.002         
next-identifier            39.646   3693     0.011         

} while (
!t.isValidIdentifier(next)
|| hop.call(bindings, next)
|| scope.hasGlobal(next)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idea: collect global references upfront and check manually instead of calling hasGlobal for each

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

each scope maintains its global references when the scope is created only (I think), and we visit scopes only once - and we transform in place. So my gut feel is that it's not possible to do better that that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wait, all scopes end up in the global... why do they need to store references?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure. scope is created (globals calculated from bindings) -> visitor applied is what I thought. I'll check though.

Copy link
Member Author

@boopathi boopathi Aug 31, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, from the timing values, the next-identifier is just 39ms (0.011 ms per call) - this includes -

let next;
            do {
              next = getNext();
            } while (
              !t.isValidIdentifier(next)
              || hop.call(bindings, next)
              || scope.hasGlobal(next)
              || scope.hasReference(next)
            );

So this is not something we need to worry about ...

@kangax kangax merged commit de07c86 into master Aug 31, 2016
@kangax kangax deleted the perf-mangle branch August 31, 2016 20:45
@hzoo hzoo added the Tag: Internal Pull Request changing project internals - code that is NOT published label Aug 31, 2016
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Tag: Internal Pull Request changing project internals - code that is NOT published
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants