Truncate the number of rows displayed on about:profiler.

Truncate the number of rows displayed on about:profiler.

Rather than displaying the full table of data (which can be quite large), we instead display only 30 (when ungrouped), or just 10 when grouped.

This speeds up the rendering of the page since a much smaller number of rows need to be drawn. It also helps reduce the clutter when grouping.

The hidden rows can be displayed with the click of a button.

BUG=100992
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=110545

[jar (doing other things), 2011-11-17 14:18:40]

LGTM.  I'll be super glad to see this feature land too! ;-)

Comments below that are more like nits.  I would suggest you consider changing
some of the limits as per discussion below.  The code changes (if any) would
best be done in another CL, so that I can get to play with this feature sooner
;-).

http://codereview.chromium.org/8574051/diff/1003/chrome/browser/resources/pro...
File chrome/browser/resources/profiler.js (right):

http://codereview.chromium.org/8574051/diff/1003/chrome/browser/resources/pro...
chrome/browser/resources/profiler.js:549: var INITIAL_UNGROUPED_ROW_LIMIT = 30;
Excellent! You beat me to the punch!  I was going to suggest the larger limit
when there is no grouping. :-)

This seems like a fair number.  You might even make it a tad larger (40?), but
given the rate of drop-off when sorted... thas may be more than enough.

http://codereview.chromium.org/8574051/diff/1003/chrome/browser/resources/pro...
chrome/browser/resources/profiler.js:554: var INITIAL_GROUP_ROW_LIMIT = 10;
I'm suspicious that this should be lower.  IMO, it may well be zero.  This will
allow you to get to the grouped item you care about, and then expand only those
items.

Especially problematic is the case where they group (for example) on function
name etc.  You'll se that the groups are so plentiful, and so short, that you'll
lose your perf gain :-(.

http://codereview.chromium.org/8574051/diff/1003/chrome/browser/resources/pro...
chrome/browser/resources/profiler.js:560: var LIMIT_INCREMENT = 10;
IF you do go for the smaller initial limit... then this might be set larger,
kindred to you ungrouped limit.

http://codereview.chromium.org/8574051/diff/1003/chrome/browser/resources/pro...
chrome/browser/resources/profiler.js:919: showNoneHandler);
nit: Not critical to this CL: Sometimes when I see this many args, I start to
think about passing a class with members.

http://codereview.chromium.org/8574051/diff/1003/chrome/browser/resources/pro...
chrome/browser/resources/profiler.js:1397:
this.changeGroupDisplayLimit_.bind(this, k, -Infinity));
As used here... the arguments are real clear... but still the correctness of the
order is hard to see with unnamed args that are this plentiful.

[eroman, 2011-11-17 19:06:51]

thanks for the review!

http://codereview.chromium.org/8574051/diff/1003/chrome/browser/resources/pro...
File chrome/browser/resources/profiler.js (right):

http://codereview.chromium.org/8574051/diff/1003/chrome/browser/resources/pro...
chrome/browser/resources/profiler.js:554: var INITIAL_GROUP_ROW_LIMIT = 10;
On 2011/11/17 14:18:40, jar wrote:
> I'm suspicious that this should be lower.  IMO, it may well be zero.  This
will
> allow you to get to the grouped item you care about, and then expand only
those
> items.
> 
> Especially problematic is the case where they group (for example) on function
> name etc.  You'll se that the groups are so plentiful, and so short, that
you'll
> lose your perf gain :-(.

It should be easy to change these policies as we play around with it. I'll leave
it as-is for now, we can update later.

http://codereview.chromium.org/8574051/diff/1003/chrome/browser/resources/pro...
chrome/browser/resources/profiler.js:919: showNoneHandler);
On 2011/11/17 14:18:40, jar wrote:
> nit: Not critical to this CL: Sometimes when I see this many args, I start to
> think about passing a class with members. 

I fully agree, the arguments here are getting unwiedly.

I'll send a follow-up CL to fix this (I don't want to do it in this CL since all
the code moves will be confusing).

http://codereview.chromium.org/8574051/diff/1003/chrome/browser/resources/pro...
chrome/browser/resources/profiler.js:1397:
this.changeGroupDisplayLimit_.bind(this, k, -Infinity));
On 2011/11/17 14:18:40, jar wrote:
> As used here... the arguments are real clear... but still the correctness of
the
> order is hard to see with unnamed args that are this plentiful.

I will fix this in a followup.