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

Slow state registration #3325

Closed
hworld opened this issue Feb 8, 2017 · 4 comments
Closed

Slow state registration #3325

hworld opened this issue Feb 8, 2017 · 4 comments

Comments

@hworld
Copy link

hworld commented Feb 8, 2017

Hey there, I've upgraded to [email protected]. Really love the new changes! Only one thing I'm having an issue with. I have a pretty big site that registers quite a few states in the config phase. I've noticed that with the new router my state registration is really really slow. It's actually taking 3.5s on my computer just to register states. It's showing nothing while this happens since it can't even route until the states are registered.

You can see the code for my states/routes here: https://github.com/gamejolt/gamejolt/tree/master/src/app/views They're in the -state.js/ts files. Nothing too fancy, I think. Just URL-based states.

I profiled the code in Chrome to get a CPU profile of the start of the page. I noticed there were two methods that were taking most of the time. I was able to get it down to ~50ms from 3.5s by just commenting them out. You can see the two changes here: https://github.com/gamejolt/gamejolt/blob/cb8b65838f4f544f042c9c3df8385f5f29c8845f/src/lib/ui-router.js Just search for @MODIFY.

I'm not very comfortable modifying the code, so I'm not totally sure what I'm changing here. The change here: https://github.com/gamejolt/gamejolt/blob/cb8b65838f4f544f042c9c3df8385f5f29c8845f/src/lib/ui-router.js#L4729 seems to be taking the majority of the time. I logged out the matches and it was always undefined so I kept that commented out. Sadly the change at https://github.com/gamejolt/gamejolt/blob/cb8b65838f4f544f042c9c3df8385f5f29c8845f/src/lib/ui-router.js#L3737 broke my states so I had to revert it. The second change was still taking about 1-1.5s of the total time of the slow registration. It seems to be sorting the states so it can match on them properly? Is there a way to do this after the main states are registered for the app in the config phase instead of doing it every time a state is registered?

I'll be working on trying to figure out how to hack the perf for the second change I made, so the site may be changed once you get to this, but you can try to grab a profile here: https://gamejolt.com

Willing to help out in any way I can since it really affects the performance of the initial load of the site. Thanks!

Edit: To add a little more info, there is currently 120 states registered on start up.

@hworld
Copy link
Author

hworld commented Feb 8, 2017

Just pushed out a new change that seems to sort the routes correctly but delays it until the "match" against all states. This is still a bit slow (10-50ms) each time it needs to route to a new page. But it seems better to eat the cost each route change vs such a large load time before showing anything.

Here's the relevant commit: gamejolt/gamejolt@535fb9f

I'm sure I broke something else while doing this, but I have no clue. =P

@tenxliviu
Copy link

+1, this needs to be fixed before a stable release. State registration shouldn't be this slow, 200-300 states isn't that big of a deal for a complex app.

After investigating the code I've found a workaround without actually changing anything in the angular ui source code.

You need to replace the sort function on the router with an empty function before starting to register states. After every state has been registered, you need to add the correct sort function back and execute it.

    let slowSort;
    app.config(['$urlRouterProvider', function ($urlRouterProvider) {
        slowSort = $urlRouterProvider._urlRouter.sort;
        $urlRouterProvider._urlRouter.sort = function(){};
    }]);

    app.config(['$stateProvider', function ($stateProvider) {
        //state definitions go in here
        //$stateProvider.state(.....)
    }]);

    app.config(['$urlRouterProvider', function ($urlRouterProvider) {
        $urlRouterProvider._urlRouter.sort = slowSort;
        $urlRouterProvider._urlRouter.sort();
    }]);

@Lukinoh
Copy link

Lukinoh commented Feb 27, 2017

Related to #3274 I think

@christopherthielen
Copy link
Contributor

christopherthielen commented Mar 17, 2017

Closing as duplicate of #3274

This is fixed in master and will be released in rc.2

The analysis is correct, thank you for doing that. The perf regression is due to sorting each time a state is registered. Sorting is an expensive task. In master sorting is deferred till Just In Time (wait until the sorted data is used for the first time). I also reduced the cost of sorting in general. rc.2 will be the same perf as ui-router 0.2.x.

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

4 participants