New binary size tool visualization options.

New binary size tool visualization options.

BUG=
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=264270

[Andrew Hayden (chromium.org), 2014-04-16 15:27:42]

Ready!

[Andrew Hayden (chromium.org), 2014-04-16 15:38:20]

See example at:
http://andrewhayden.github.io/new_binary_size_example/

[tfarina, 2014-04-16 16:11:59]

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/READM...
File tools/binary_size/README.txt (right):

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/READM...
tools/binary_size/README.txt:120:
https://code.google.com/p/chromium/issues/entry?labels=Binary-Size-Tool
s/Binary-Size-Tool/Toosl-BinarySize ?

[Andrew Hayden (chromium.org), 2014-04-16 16:14:46]

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/READM...
File tools/binary_size/README.txt (right):

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/READM...
tools/binary_size/README.txt:120:
https://code.google.com/p/chromium/issues/entry?labels=Binary-Size-Tool
On 2014/04/16 16:12:00, tfarina wrote:
> s/Binary-Size-Tool/Toosl-BinarySize ?

Done.

[bulach, 2014-04-16 16:32:15]

lgtm (sorry, I think I got some comments spread across multiple patchsets..).

this looks great overall, thanks! just minor nits and suggestions (mostly:
indent and spaces around operators)...

I didn't review all the js logic too deeply, but I assume the results that it
will display will be valuable to actually reduce the binary size! thanks!

https://codereview.chromium.org/231803002/diff/160001/tools/binary_size/exper...
File tools/binary_size/experimental_template/D3SymbolTreeMap.js (right):

https://codereview.chromium.org/231803002/diff/160001/tools/binary_size/exper...
tools/binary_size/experimental_template/D3SymbolTreeMap.js:40: for (var
x=asString.length - 1; x >= 0; x--) {
nit:s/x=as/x = as/

https://codereview.chromium.org/231803002/diff/160001/tools/binary_size/exper...
tools/binary_size/experimental_template/D3SymbolTreeMap.js:58: if (num >=
1024*1024*1024) {
nit: spaces around * here and below

https://codereview.chromium.org/231803002/diff/160001/tools/binary_size/exper...
tools/binary_size/experimental_template/D3SymbolTreeMap.js:73:
D3SymbolTreeMap._NM_SYMBOL_TYPES = 'ABbCDdGgiNpRrSsTtUuVvWw@-?';
nit: rather than duplicate, perhaps define below:
D3SymbolTreeMap._NM_SYMBOL_TYPES = []
for (var k in D3SymbolTreeMap._NM_SYMBOL_TYPE_DESCRIPTIONS)
  D3SymbolTreeMap._NM_SYMBOL_TYPES.push(k);


...or just use the "for (... in ...)" where needed..

https://codereview.chromium.org/231803002/diff/160001/tools/binary_size/exper...
tools/binary_size/experimental_template/D3SymbolTreeMap.js:135: for (var x=0;
x<D3SymbolTreeMap._NM_SYMBOL_TYPES.length; x++) {
nit: s/x=0/x = 0/

also, as above, this could be:
var index;
for (var k in D3SymbolTreeMap._NM_SYMBOL_TYPE_DESCRIPTIONS) {
  ...
  ++index;
}

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/run_b...
File tools/binary_size/run_binary_size_analysis.py (right):

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/run_b...
tools/binary_size/run_binary_size_analysis.py:93: 
nit: another \n

also, _MkChuld

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/run_b...
tools/binary_size/run_binary_size_analysis.py:100: if child == None:
nit: 
if not child:

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/run_b...
tools/binary_size/run_binary_size_analysis.py:104: 
nit: another \n

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/templ...
File tools/binary_size/template/D3SymbolTreeMap.js (right):

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/templ...
tools/binary_size/template/D3SymbolTreeMap.js:10: this._mapContainer =
undefined;
nit: I think indent is 2 rather than 4

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/templ...
tools/binary_size/template/D3SymbolTreeMap.js:205: if (datum.children) for (var
i=0; i<datum.children.length; i++)
nit: space around = and <

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/templ...
tools/binary_size/template/D3SymbolTreeMap.js:216: for (var i=0; i<stack.length;
i++) {
nit: space = and <

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/templ...
tools/binary_size/template/D3SymbolTreeMap.js:229: } else for (var i=0;
i<node.children.length; i++) {
nit: space = and <

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/templ...
tools/binary_size/template/D3SymbolTreeMap.js:374:
//this._showHoverMask(selection);
nit: remove? (and 379...)

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/templ...
tools/binary_size/template/D3SymbolTreeMap.js:447: .style('z-index',
function(datum) { return (datum.id*2)+1; })
nit: space

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/templ...
tools/binary_size/template/D3SymbolTreeMap.js:747: /*
nit: remove? (ditto for 769-777 block below)

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/templ...
File tools/binary_size/template/test-data-generator.html (right):

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/templ...
tools/binary_size/template/test-data-generator.html:11: return
Math.round(Math.random()*max);
nit: indent

[Andrew Hayden (chromium.org), 2014-04-16 17:40:15]

https://codereview.chromium.org/231803002/diff/160001/tools/binary_size/exper...
File tools/binary_size/experimental_template/D3SymbolTreeMap.js (right):

https://codereview.chromium.org/231803002/diff/160001/tools/binary_size/exper...
tools/binary_size/experimental_template/D3SymbolTreeMap.js:40: for (var
x=asString.length - 1; x >= 0; x--) {
On 2014/04/16 16:32:16, bulach wrote:
> nit:s/x=as/x = as/

Done.

https://codereview.chromium.org/231803002/diff/160001/tools/binary_size/exper...
tools/binary_size/experimental_template/D3SymbolTreeMap.js:58: if (num >=
1024*1024*1024) {
On 2014/04/16 16:32:16, bulach wrote:
> nit: spaces around * here and below

Done.

https://codereview.chromium.org/231803002/diff/160001/tools/binary_size/exper...
tools/binary_size/experimental_template/D3SymbolTreeMap.js:73:
D3SymbolTreeMap._NM_SYMBOL_TYPES = 'ABbCDdGgiNpRrSsTtUuVvWw@-?';
On 2014/04/16 16:32:16, bulach wrote:
> nit: rather than duplicate, perhaps define below:
> D3SymbolTreeMap._NM_SYMBOL_TYPES = []
> for (var k in D3SymbolTreeMap._NM_SYMBOL_TYPE_DESCRIPTIONS)
>   D3SymbolTreeMap._NM_SYMBOL_TYPES.push(k);
> 
> 
> ...or just use the "for (... in ...)" where needed..

Done.

https://codereview.chromium.org/231803002/diff/160001/tools/binary_size/exper...
tools/binary_size/experimental_template/D3SymbolTreeMap.js:135: for (var x=0;
x<D3SymbolTreeMap._NM_SYMBOL_TYPES.length; x++) {
On 2014/04/16 16:32:16, bulach wrote:
> nit: s/x=0/x = 0/
> 
> also, as above, this could be:
> var index;
> for (var k in D3SymbolTreeMap._NM_SYMBOL_TYPE_DESCRIPTIONS) {
>   ...
>   ++index;
> }

Done.

[Andrew Hayden (chromium.org), 2014-04-16 17:53:21]

Everything except indentation.

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/run_b...
File tools/binary_size/run_binary_size_analysis.py (right):

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/run_b...
tools/binary_size/run_binary_size_analysis.py:93: 
On 2014/04/16 16:32:16, bulach wrote:
> nit: another \n
> 
> also, _MkChuld

Done.

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/run_b...
tools/binary_size/run_binary_size_analysis.py:100: if child == None:
On 2014/04/16 16:32:16, bulach wrote:
> nit: 
> if not child:

Done.

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/run_b...
tools/binary_size/run_binary_size_analysis.py:104: 
On 2014/04/16 16:32:16, bulach wrote:
> nit: another \n

Done throughout

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/templ...
File tools/binary_size/template/D3SymbolTreeMap.js (right):

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/templ...
tools/binary_size/template/D3SymbolTreeMap.js:205: if (datum.children) for (var
i=0; i<datum.children.length; i++)
On 2014/04/16 16:32:16, bulach wrote:
> nit: space around = and <

Done.

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/templ...
tools/binary_size/template/D3SymbolTreeMap.js:216: for (var i=0; i<stack.length;
i++) {
On 2014/04/16 16:32:16, bulach wrote:
> nit: space = and <

Done.

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/templ...
tools/binary_size/template/D3SymbolTreeMap.js:229: } else for (var i=0;
i<node.children.length; i++) {
On 2014/04/16 16:32:16, bulach wrote:
> nit: space = and <

Done.

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/templ...
tools/binary_size/template/D3SymbolTreeMap.js:374:
//this._showHoverMask(selection);
On 2014/04/16 16:32:16, bulach wrote:
> nit: remove? (and 379...)

Removed the entire hovermask feature, which never looked good.

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/templ...
tools/binary_size/template/D3SymbolTreeMap.js:447: .style('z-index',
function(datum) { return (datum.id*2)+1; })
On 2014/04/16 16:32:16, bulach wrote:
> nit: space

Done.

https://codereview.chromium.org/231803002/diff/180001/tools/binary_size/templ...
tools/binary_size/template/D3SymbolTreeMap.js:747: /*
On 2014/04/16 16:32:16, bulach wrote:
> nit: remove? (ditto for 769-777 block below)

Done.

[Andrew Hayden (chromium.org), 2014-04-16 18:20:09]

Indentation fixed, ready to go!

[Andrew Hayden (chromium.org), 2014-04-16 18:20:32]

The CQ bit was checked by andrewhayden@chromium.org

[commit-bot: I haz the power, 2014-04-16 18:21:03]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/andrewhayden@chromium.org/231803002/23...

[commit-bot: I haz the power, 2014-04-16 18:53:20]

Change committed as 264270