Lightweight assets/improve FF performance

Lightweight assets/improve FF performance

Some lighter weight assets for service modules were implemented, via Matt C.  Additionally, profiling showed that a lot of slowness for FireFox was caused by FF internals being slow (namely <node>.setAttribute() and <node>.getClientRects()).  While we can't fix those being slow, we can reduce the number of times they're used.  This is done by modifying attributes on only relevant relation lines when a service is dragged (rather than redrawing or modifying all lines).  This results in fewer calls to both of those internals, and speeds up animation in FireFox.

https://code.launchpad.net/~makyo/juju-gui/replace-service-module/+merge/132938

(do not edit description out of merge proposal)

[matthew.scott, 2012-11-05 18:04:29]

Please take a look.

[gary.poster, 2012-11-05 18:43:20]

+1.  Thank you.

Gary

[benjamin.saller, 2012-11-05 18:57:02]

The impl looks fine but having tested under FF the new asset doesn't seem to
perform any better. This is after manually doing make appcache-force so I'm
pretty sure its the current asset. 

I'm not sure we should land this as it doesn't yield the benefit we hoped for.

[matthew.scott, 2012-11-07 22:41:28]

Please take a look.

[benjamin.saller, 2012-11-09 18:38:31]

This looks good to me, I had a suggestion to test below, but either way on that
one.

This doesn't fix the speed issues in FF for me, but it does improve the
situation. 

Thanks.

https://codereview.appspot.com/6826057/diff/3001/app/views/environment.js
File app/views/environment.js (right):

https://codereview.appspot.com/6826057/diff/3001/app/views/environment.js#new...
app/views/environment.js:528: 
I know we talked about adding a method like this, really happy to see you added
it, thanks.

https://codereview.appspot.com/6826057/diff/3001/app/views/environment.js#new...
app/views/environment.js:645: //  .one('text').getClientRect() || {width:
0}).width + 10;
Can we specify it as d.display_name.length + 10 + 'em'? I didn't test it, but em
should be font size relative if it works.

[gary.poster, 2012-11-09 21:02:39]

This makes FF usable for me both on my relatively new desktop and my 3+ year old
laptop.  Amazing!  Thank you.

I have a few trivial comments/suggestions/requests.  Approved.

Gary

https://codereview.appspot.com/6826057/diff/3001/app/modules.js
File app/modules.js (right):

https://codereview.appspot.com/6826057/diff/3001/app/modules.js#newcode14
app/modules.js:14: 'fullpath': '/juju-ui/assets/javascripts/d3.v2.js'
I guess this is good for thiago's branch, which will be minifying everything
anyway, so that the debug story is nice.  Is that why you did it?

https://codereview.appspot.com/6826057/diff/3001/app/views/environment.js
File app/views/environment.js (right):

https://codereview.appspot.com/6826057/diff/3001/app/views/environment.js#new...
app/views/environment.js:645: //  .one('text').getClientRect() || {width:
0}).width + 10;
I'm -1 on checking in commented-out code to trunk; if you add an explanatory
comment as to why you have left this in but commented out, I might feel better
about it. :-)

[matthew.scott, 2012-11-09 21:21:32]

Thanks

https://codereview.appspot.com/6826057/diff/3001/app/modules.js
File app/modules.js (right):

https://codereview.appspot.com/6826057/diff/3001/app/modules.js#newcode14
app/modules.js:14: 'fullpath': '/juju-ui/assets/javascripts/d3.v2.js'
Left over from profiling; back to minified.

On 2012/11/09 21:02:40, gary.poster wrote:
> I guess this is good for thiago's branch, which will be minifying everything
> anyway, so that the debug story is nice.  Is that why you did it?

https://codereview.appspot.com/6826057/diff/3001/app/views/environment.js
File app/views/environment.js (right):

https://codereview.appspot.com/6826057/diff/3001/app/views/environment.js#new...
app/views/environment.js:645: //  .one('text').getClientRect() || {width:
0}).width + 10;
Unfortunately not, as we use this attribute on line 649, expecting it to be
pixels, as we don't know how wide an em is.

On 2012/11/09 18:38:31, benjamin.saller wrote:
> Can we specify it as d.display_name.length + 10 + 'em'? I didn't test it, but
em
> should be font size relative if it works.

https://codereview.appspot.com/6826057/diff/3001/app/views/environment.js#new...
app/views/environment.js:645: //  .one('text').getClientRect() || {width:
0}).width + 10;
Ooops!  Done!

On 2012/11/09 21:02:40, gary.poster wrote:
> I'm -1 on checking in commented-out code to trunk; if you add an explanatory
> comment as to why you have left this in but commented out, I might feel better
> about it. :-)

[matthew.scott, 2012-11-09 21:27:31]

*** Submitted:

Lightweight assets/improve FF performance

Some lighter weight assets for service modules were implemented, via Matt C. 
Additionally, profiling showed that a lot of slowness for FireFox was caused by
FF internals being slow (namely <node>.setAttribute() and
<node>.getClientRects()).  While we can't fix those being slow, we can reduce
the number of times they're used.  This is done by modifying attributes on only
relevant relation lines when a service is dragged (rather than redrawing or
modifying all lines).  This results in fewer calls to both of those internals,
and speeds up animation in FireFox.

R=gary.poster, benjamin.saller
CC=
https://codereview.appspot.com/6826057