Performance Optimization: Defer RouteMaker Instantiation in URL Addon #252

Merged
merged 2 commits into from Jul 17, 2012

Projects

None yet

4 participants

@carystanley

The URL addon is one of the default addons that gets added to the Action Context whether or not that addon is actually used. The URL addon builds out a RouteMaker in it's constructor, which for our application is roughly 4ms for each instantiation. With 20 mojits in the main loading this change of deferring this part of the addons instantiation saves roughly 72ms for us.

@drewfish
Yahoo Inc. member

nice

@rwaldura

Very.

@drewfish, Cary: any possible downsides to this patch?

@rwaldura

Related: I heard from another client (Stars, I think) that the more routes you have, the slower it gets.
I'll dig up the reference.

@carystanley

There are not any downsides, it just defers the work of building the RouteMaker only in contexts where it is actually used.

For applications like ours with multiple routes and multiple Mojits, but where the url addon is only used a few times there is significant savings.

@carystanley

Renaud, this would help "Star", presently the RouteMaker is instantiated once at the Route Middleware and once and Deploy if you are deploy client-side and then once for every Mojit called (every ActionContext). This would remove the the instantiation for each ActionContext and only instantiate on the first call made to ac.url per ActionContext.

@mojit0

Currently testing this for merge and inclusion in the next release.

@mojit0

Unfortunately this patch doesn't pass jslint due to '' usage. The patch should be updated to either remove the '' (which is probably the best solution given that we don't have a coding standard which promotes '_' use), or to add the jslint directive 'nomen' as in /jslint nomen:true/ to the top of the file. If you're able to make these changes and run the result through jslint cleanly that'll be fastest. Otherwise let us know and we'll do it on our end...it'll just take a tiny bit more time :)

@carystanley

I have removed the use of underscores in the patch, hopefully that should resolve things. The following still appears:

1- 'window' was used before it was defined.
if (typeof window !== 'undefined') { // Line 84, Pos 24

But this was an issue before my changes

@mojit0 mojit0 merged commit 52432e5 into yahoo:develop Jul 17, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment