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

Performance of new Order URL Matching Rules by priority, not registration order #27

Closed
towu opened this issue Jan 14, 2017 · 2 comments
Closed

Comments

@towu
Copy link

towu commented Jan 14, 2017

The new matching functions seem to have some serious performance impact if you use many states:

Plunkr with 500 states:

Plunkr: pre 3.0.0 (beta.3) Fast URL match: angular.run reached in 2 seconds
http://plnkr.co/edit/BXsCPsh5qLiVatmavtiO?p=preview

Plunkr: 3.0.0 (rc.1) Slow URL match: angular.run reached in 25 seconds
http://plnkr.co/edit/0vb4ReYiabWsZprCrk4F?p=preview

I do use a lot of states (angular 1, 0.3.X migration) without priority and noticed this performance issue in my application.

@christopherthielen
Copy link
Member

Appreciate the report and plunkers. 25 seconds is way longer than acceptable, hah! I'll see if I can figure out why the change in performance is happening.

The "by priority" match shouldn't cause a change by a factor of 10. In the worst case, it should be 2x average due to less short-circuiting.

christopherthielen added a commit that referenced this issue Jan 31, 2017
- extend: Use native ES6 `Object.assign` if available
- extend: Use nested for loop for non-native polyfill
- defaults: Simplify logic
- pick / omit: avoid v8 deopt. simplify. remove varargs support
- arrayTuples: unroll for arguments.length === 1, 2, 3, 4

Related to #27
Related to angular-ui/ui-router#3274
christopherthielen added a commit that referenced this issue Jan 31, 2017
…r.find()` misses

The `.find()` function is pretty hot/frequently used.
When possible, a state is found using a property look up `this._states[name]`
If a state isn't found using an exact match, another attempt is made to do a glob match.
The glob match allows a future state such as `future.state.**` to be matched for any child such as `future.state.child.nested`.

This causes unnecessary overhead especially during bootup and registration.
Before registering a state, we first check to see if it already exists.
This change does three things:

1) Only state names that *might* be a glob are tested
2) The Globs for states with glob-like names are cached
3) Do not use globs in `.register()` when checking for "State foo is already defined"

Related to #27
Related to angular-ui/ui-router#3274
@christopherthielen
Copy link
Member

screen shot 2017-01-31 at 11 26 27 am


screen shot 2017-01-31 at 11 28 39 am


screen shot 2017-01-31 at 11 37 48 am

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants