Added ProfileView object for performing sorting, searching and filtering operations on a profile.

Added ProfileView object for performing sorting, searching and filtering operations on a profile.

It will be used both in the new tickprocessor and Dev Tools profiler.

Committed: http://code.google.com/p/v8/source/detail?r=1786

[Søren Thygesen Gjesse, 2009-04-24 10:55:20]

LGTM

http://codereview.chromium.org/92120/diff/1/5
File tools/profileview.js (right):

http://codereview.chromium.org/92120/diff/1/5#newcode54
Line 54: callTree.traverse(function(node, viewParent) {
I woud prefer to have this function defined as an inner function in
devtools.profiler.ViewBuilder.prototype.buildView instead of being defined
anonomous in the arguments list.

[Mikhail Naganov, 2009-04-24 11:34:23]

http://codereview.chromium.org/92120/diff/1/5
File tools/profileview.js (right):

http://codereview.chromium.org/92120/diff/1/5#newcode54
Line 54: callTree.traverse(function(node, viewParent) {
On 2009/04/24 10:55:20, Søren Gjesse wrote:
> I woud prefer to have this function defined as an inner function in
> devtools.profiler.ViewBuilder.prototype.buildView instead of being defined
> anonomous in the arguments list.

I see two obstacles to this. First, inside 'traverse', this function is called
within a global context (as callbacks usually do), thus it will be needed to use
an intermediate function that will reapply it in the context of an ViewBuilder
object.

Second, the visitor function initializes the 'head' variable available from its
lexical scope. I can make 'head' a property of ViewBuilder, but it seems not
very good to me, as it is a temporary var.

So, would I change a code as you propose, it will look like this:

devtools.profiler.ViewBuilder = function(samplingRate) {
   this.samplingRate = samplingRate;
   // Used inside cloningVisitor.
   this.head = null;
};

devtools.profiler.ViewBuilder.prototype.cloningVisitor = function(node,
viewParent) {
   // body of now anonymous function,
   // uses 'this' to access 'samplingRate' and 'head'.
}

...and inside devtools.profiler.ViewBuilder.prototype.buildView:

var self = this;
callTree.traverse(function(node, viewParent) { self.cloningVisitor.apply(self,
arguments); });


Frankly, I don't like it. If you don't have strong arguments for this change,
I'll better leave the code as is.