Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(8)

Issue 70013002: Support for the Linux 'perf report' and 'perf annotate' tools. (Closed)

Created:
4 years ago by Jarin
Modified:
3 years, 11 months ago
Reviewers:
bnoordhuis, danno
CC:
v8-dev
Visibility:
Public.

Description

Support for the Linux 'perf report' and 'perf annotate' tools. In this change, the support comes in two flavours: --perf_jit_prof - outputs the files in a new perf format that only works with a patched perf tool (patch obtained from Stephane Eranian). Both 'perf report' and 'perf annotate' are supported (the file format also contains the machine code). --perf_basic_prof - outputs the files in a format that the existing perf tool can consume. Only 'perf report' is supported. In both cases, we have to disable code compaction because the perf tool does not understand code relocation. (We are told that code relocation should be supported soon.) Usage: perf record -g d8 --perf_jit_prof --no_compact_code_space my.js perf report The change itself is straightforward - we simply listen to code events and write an entry to a log file for every new piece of code. I am not yet sure whether we should keep both versions or just one (and which one). My hope is the reviewers can help here. R=danno@chromium.org BUG= Committed: https://code.google.com/p/v8/source/detail?r=18034

Patch Set 1 #

Patch Set 2 : Remove forgotten printfs. #

Total comments: 12

Patch Set 3 : Minor fixes (addressing reviewers' comments) #

Patch Set 4 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+271 lines, -2 lines) Patch
M src/flag-definitions.h View 1 2 3 1 chunk +4 lines, -0 lines 0 comments Download
M src/isolate.cc View 1 2 3 1 chunk +11 lines, -1 line 0 comments Download
M src/log.h View 2 chunks +4 lines, -0 lines 0 comments Download
M src/log.cc View 1 2 4 chunks +250 lines, -0 lines 0 comments Download
M src/log-utils.h View 1 chunk +2 lines, -1 line 0 comments Download
Trybot results:

Messages

Total messages: 13 (0 generated)
Jarin
4 years ago (2013-11-12 07:13:02 UTC) #1
danno
Going in the right direction! Feedback is mostly nits... https://codereview.chromium.org/70013002/diff/40001/src/flag-definitions.h File src/flag-definitions.h (right): https://codereview.chromium.org/70013002/diff/40001/src/flag-definitions.h#newcode790 src/flag-definitions.h:790: ...
4 years ago (2013-11-13 17:18:09 UTC) #2
bnoordhuis
Left some comments. I really like this patch. Tested it with node.js master and it ...
4 years ago (2013-11-13 23:40:51 UTC) #3
Jarin
On 2013/11/13 17:18:09, danno wrote: > Going in the right direction! Feedback is mostly nits... ...
4 years ago (2013-11-14 07:51:34 UTC) #4
Jarin
Thanks for the great comments. In patch set 3, I fixed everything except the fwrite ...
4 years ago (2013-11-14 07:56:48 UTC) #5
bnoordhuis
On 2013/11/14 07:56:48, jarin wrote: > Thanks for the great comments. In patch set 3, ...
4 years ago (2013-11-14 12:02:03 UTC) #6
Jarin
On 2013/11/14 12:02:03, bnoordhuis wrote: > ... > > As to fwrite() writing fewer bytes ...
4 years ago (2013-11-14 16:04:11 UTC) #7
Sven Panne
On 2013/11/14 16:04:11, jarin wrote: > I understand what you are saying, and I appreciate ...
4 years ago (2013-11-15 07:24:32 UTC) #8
danno
Given that the write handling is no worse than what existed before for ll_prof, I'm ...
3 years, 12 months ago (2013-11-21 20:16:09 UTC) #9
Jarin
Committed patchset #4 manually as r18034 (presubmit successful).
3 years, 11 months ago (2013-11-25 06:44:30 UTC) #10
Yang
On 2013/11/25 06:44:30, jarin wrote: > Committed patchset #4 manually as r18034 (presubmit successful). Up ...
3 years, 11 months ago (2013-11-25 08:14:47 UTC) #11
bnoordhuis
Not the author but maybe I can comment. On 2013/11/25 08:14:47, Yang wrote: > Up ...
3 years, 11 months ago (2013-11-25 10:55:34 UTC) #12
Jarin
3 years, 11 months ago (2013-11-25 12:00:22 UTC) #13
Message was sent while issue was closed.
On 2013/11/25 08:14:47, Yang wrote:
> Up till now we use a wrapper script (tools/run-llprof.sh) to run perf. I
wonder
> whether you can simply update it. What's the difference between "-e cycles"
and
> "-g"? Also, the script increases the kernel limit on sampling rate so that we
> don't have missing ticks. Is this still necessary?

We are still keeping the existing ll_prof support as is for a while, at least
until the JIT support for perf-annotate makes it to the mainline Linux. (As for
the questions - Ben has answered them better than I could.)

Powered by Google App Engine
This is Rietveld efc10ee0f