catapult-bot
commented
9 years ago Closed catapult-bot closed 9 years ago
catapult-bot
commented
9 years ago catapult-bot
commented
9 years ago 
Looks like we'd want to hook rjsmin into https://github.com/google/trace-viewer/blob/master/third_party/tvcm/tvcm/generate.py ?
Since tvcm is in the third_party directory, is it from a separate repo? If so, where does the repo live?
Looks like rjsmin can be pulled from here: http://opensource.perlig.de/rjsmin/
catapult-bot
commented
9 years ago 
I've found it useful to debug traceviewer live on shipping versions of Chrome. Presumably rjsmin would make this scenario more difficult. How much do we gain by doing this? I assume that rjsmin would reduce the Chrome download size?
catapult-bot
commented
9 years ago 
Trace viewer is a non-trivial amount of the total GRD when added into chrome. Compressing it should help with that.
We'd probably also want to strip comments, extraneous white space, etc, if possible.
catapult-bot
commented
9 years ago
Okay, as long as the tradeoff is worth it (i.e.; enough savings to make it worthwhile)
catapult-bot
commented
9 years ago 
As long as we provide a build flag that enable turning minifying trace viewer on & off, I think we should be ok? Also why do we want to use this over closure's jscompiler?
catapult-bot
commented
9 years ago
This is in python which we can run on the bots. Closure isn't, so we can't run it on the bots.
I'm not sure if we need to the flag to turn minification on/off, I'm not sure how you'd even use it as this is called through the chrome build system.
catapult-bot
commented
9 years ago
I am not sure about the bot policy but does https://pypi.python.org/pypi/closure/20140110 help?
catapult-bot
commented
9 years ago
That seems more complicated then just running one done in python. What benefit would closure give us?
catapult-bot
commented
9 years ago
I suspect closure can do a better job at minifying javascript since it's supported & used by a large community.
catapult-bot
commented
9 years ago
Update on this:
1) I ran generate_about_tracing with rjsmin, closure, and no minifier. Got the following:
Unminified: 189592 about_tracing.html 1757785 about_tracing.js
rJSmin: 138269 about_tracing.html 1272878 about_tracing.js
closure: 138269 about_tracing.html 1032984 about_tracing.js
So Ned is correct that closure can do a better job at minifying JavaScript. I doubt we are going to find a minifier that does not use node or rhino which is on par with the ones which do. rJSmin is based on Douglas Crockford's jsmin from 2003. It just runs regular expressions to remove comments and whitespace. Still, it minifies quite well.
To Bruce's point that it's hard to debug minified JS, I think rJSmin actually gives us an advantage here. Since it's not smart enough to rename local variables, shuffle code, etc, it should work quite well with the "{}" button in devtools which formats JavaScript. Of course there will be no comments.
What is the best way to test my rjsmin-minified files?
catapult-bot
commented
9 years ago 
Wow! this is neat!
rjsmin seems extremely promising. Thats a serious size savings.
As far as testing, I think the only real easy way is to try a build with this minification turned on. You might be able to run the tests somehow by vulcanizing the test files together but itd require lots of hacks.. We could discuss offline whether its necessary to do that, and I could give you some pointers if its useful.
As far as debugging goes, we try to rarely debug about:Tracing embedded in chrome. The purpose of the devserver is specifically so that we can do whatever we want with the outpt code. :)
catapult-bot
commented
9 years ago
This is all landed.
One note: When I built the ToT trace-viewer into chromium, the record button threw a JS error. This also happens WITHOUT any of my minification patches, so I believe it's unrelated to my changes. I'm headed out on vacation and don't have time to bisect the cause, unfortunately. Viewing a trace works correctly with minification.
For debugging, since there is no renaming going on, you can hit the "{}" button in devtools and get decently formatted code, but it's missing comments.
catapult-bot
commented
9 years ago
I added the step of vulcanizing the trace-viewer & reporting size in travis build so we can keep track of size changes per submit.
However, since Annies' fix, size is still pretty large:
4.80s$ ./vulcanize_trace_viewer --config=chrome
The command "./vulcanize_trace_viewer --config=chrome" exited with 0.
4.72s$ ./vulcanize_trace_viewer --config=full
The command "./vulcanize_trace_viewer --config=full" exited with 0.
0.01s$ stat -c%s bin/trace_viewer_chrome.html
1357449
The command "stat -c%s bin/trace_viewer_chrome.html" exited with 0.
0.01s$ stat -c%s bin/trace_viewer_full.html
1357454
catapult-bot
commented
9 years ago
Not clear why this should be reopened. We're using Rjsmin, so this should probably get closed.
If we want another new bug about size of trace viewer, thats fine... I think its good having a discussion about how to be smaller but also alternatives like how to deal with size. Lots of options here. But better on a fresh thread.
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/build/scripts/rjsmin.py&q=rjsmin&sq=package:chromium&type=cshttps://codereview.chromium.org/252633006&l=22