Issue 1547023: C++ profiler: publish the new API, make compatible with WebKit / Chromium.

Issue 1547023: C++ profiler: publish the new API, make compatible with WebKit / Chromium. (Closed)

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

Modified:
9 years, 7 months ago

Reviewers:
Søren Thygesen Gjesse

CC:
v8-dev

Visibility:
Public.

Description

C++ profiler: publish the new API, make compatible with WebKit / Chromium. I succeeded at connecting the new implementation to Chromium, this commit includes required (although, not all) adjustments. Committed: http://code.google.com/p/v8/source/detail?r=4349

Patch Set 1 #

Total comments: 6

Patch Set 2 : comments addressed #

Created: 10 years, 8 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+378 lines, -24 lines)			Patch
A	include/v8-profiler.h	View	1	1 chunk	+176 lines, -0 lines	0 comments	Download
M	src/api.cc	View		3 chunks	+127 lines, -0 lines	0 comments	Download
M	src/cpu-profiler.h	View		1 chunk	+3 lines, -0 lines	0 comments	Download
M	src/cpu-profiler.cc	View		5 chunks	+24 lines, -6 lines	0 comments	Download
M	src/flag-definitions.h	View		1 chunk	+2 lines, -0 lines	0 comments	Download
M	src/platform-linux.cc	View		1 chunk	+0 lines, -4 lines	0 comments	Download
M	src/profile-generator.h	View		5 chunks	+8 lines, -2 lines	0 comments	Download
M	src/profile-generator.cc	View	1	9 chunks	+36 lines, -11 lines	0 comments	Download
M	src/profile-generator-inl.h	View		1 chunk	+2 lines, -1 line	0 comments	Download

Messages

Total messages: 3 (0 generated)

Expand Messages | Collapse Messages

Søren Thygesen Gjesse

LGTM http://codereview.chromium.org/1547023/diff/1/2 File include/v8-profiler.h (right): http://codereview.chromium.org/1547023/diff/1/2#newcode159 include/v8-profiler.h:159: static void StartProfiling(Handle<String> title = Handle<String>()); Should this ...

10 years, 8 months ago (2010-04-06 14:25:38 UTC) #2

mnaganov (inactive)

10 years, 8 months ago (2010-04-06 14:46:39 UTC) #3

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

http://codereview.chromium.org/1547023/diff/1/2#newcode159
include/v8-profiler.h:159: static void StartProfiling(Handle<String> title =
Handle<String>());
On 2010/04/06 14:25:38, Søren Gjesse wrote:
> Should this function maybe mandate passing a title?

OK.

http://codereview.chromium.org/1547023/diff/1/4
File src/cpu-profiler.cc (right):

http://codereview.chromium.org/1547023/diff/1/4#newcode373
src/cpu-profiler.cc:373: "RegExp: ",
On 2010/04/06 14:25:38, Søren Gjesse wrote:
> Do we need to pass the "RegExp: " prefix when the event is already RegExp
> specific?

This simplifies name getter logic: no need to check the event tag.

http://codereview.chromium.org/1547023/diff/1/9
File src/profile-generator.cc (right):

http://codereview.chromium.org/1547023/diff/1/9#newcode511
src/profile-generator.cc:511: *entry++ = FLAG_prof_browser_mode ? program_entry_
: NULL;
On 2010/04/06 14:25:38, Søren Gjesse wrote:
> Please make a comment here about how browser mode differs.

Done.

Expand Messages | Collapse Messages