Issue 2822009: Heap profiler: publish API and add test.

Issue 2822009: Heap profiler: publish API and add test. (Closed)

Created:
10 years, 6 months ago by mnaganov (inactive)

Modified:
9 years, 7 months ago

Reviewers:
Søren Thygesen Gjesse

CC:
v8-dev

Visibility:
Public.

Description

Heap profiler: publish API and add test. Committed: http://code.google.com/p/v8/source/detail?r=4864

Patch Set 2 : comments addressed #

Created: 10 years, 6 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+611 lines, -16 lines)			Patch
M	include/v8-profiler.h	View	1	1 chunk	+139 lines, -0 lines	0 comments	Download
M	src/api.cc	View	1	2 chunks	+191 lines, -0 lines	0 comments	Download
M	src/heap-profiler.h	View		1 chunk	+24 lines, -1 line	0 comments	Download
M	src/heap-profiler.cc	View		2 chunks	+70 lines, -1 line	0 comments	Download
M	src/profile-generator.h	View	1	2 chunks	+9 lines, -9 lines	0 comments	Download
M	src/profile-generator.cc	View		3 chunks	+14 lines, -5 lines	0 comments	Download
M	src/v8.cc	View		3 chunks	+4 lines, -0 lines	0 comments	Download
M	test/cctest/test-heap-profiler.cc	View		2 chunks	+160 lines, -0 lines	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Søren Thygesen Gjesse

LGTM http://codereview.chromium.org/2822009/diff/1/2 File include/v8-profiler.h (right): http://codereview.chromium.org/2822009/diff/1/2#newcode202 include/v8-profiler.h:202: /** Returns edge type (see Type). */ Type ...

10 years, 6 months ago (2010-06-15 10:48:51 UTC) #2

mnaganov (inactive)

10 years, 6 months ago (2010-06-15 11:34:03 UTC) #3

http://codereview.chromium.org/2822009/diff/1/2
File include/v8-profiler.h (right):

http://codereview.chromium.org/2822009/diff/1/2#newcode202
include/v8-profiler.h:202: /** Returns edge type (see Type). */
On 2010/06/15 10:48:51, Søren Gjesse wrote:
> Type -> HeapGraphEdge::Type?

Done.

http://codereview.chromium.org/2822009/diff/1/2#newcode244
include/v8-profiler.h:244: JS_OBJECT = 3,  // A JS object (except for arrays and
strings).
On 2010/06/15 10:48:51, Søren Gjesse wrote:
> JS_OBJECT -> just OBJECT?

Done.

http://codereview.chromium.org/2822009/diff/1/2#newcode249
include/v8-profiler.h:249: /** Returns node type (see Type). */
On 2010/06/15 10:48:51, Søren Gjesse wrote:
> Type -> HeapGraphNode::Type?

Done.

http://codereview.chromium.org/2822009/diff/1/2#newcode255
include/v8-profiler.h:255: * string value, or an empty string.
On 2010/06/15 10:48:51, Søren Gjesse wrote:
> Empty string for code?

Added. Thanks to your comment, I also discovered that I didn't set names for
Scripts and SFIs. Fixed.

http://codereview.chromium.org/2822009/diff/1/2#newcode257
include/v8-profiler.h:257: Handle<String> GetSourceName() const;
On 2010/06/15 10:48:51, Søren Gjesse wrote:
> GetSourceName() -> GetName()?

Done.

http://codereview.chromium.org/2822009/diff/1/2#newcode267
include/v8-profiler.h:267: * that will be reclaimed having this node deleted.
On 2010/06/15 10:48:51, Søren Gjesse wrote:
> deleted -> collected.

Done.

Expand Messages | Collapse Messages