Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(614)

Issue 624443005: Revert of Extend CPU profiler with mapping ticks to source lines (Closed)

Created:
6 years, 2 months ago by yurys
Modified:
6 years, 2 months ago
CC:
v8-dev, Paweł Hajdan Jr.
Project:
v8
Visibility:
Public.

Description

Revert of Extend CPU profiler with mapping ticks to source lines (patchset #3 id:40001 of https://codereview.chromium.org/616963005/) Reason for revert: It broke layout test fast/events/window-onerror-02.html, error column reported by window.onerror is now wrong (I believe it is because of the change in full-codegen): http://build.chromium.org/p/client.v8/builders/V8-Blink%20Linux%2064%20%28dbg%29/builds/652 Original issue's description: > Extend CPU profiler with mapping ticks to source lines > > The idea behind of this solution is to use the existing "relocation info" instead of consumption the CodeLinePosition events emitted by the V8 compilers. > During generation code and relocation info are generated simultaneously. > When code generation is done you each code object has associated "relocation info". > Relocation information lets V8 to mark interesting places in the generated code: the pointers that might need to be relocated (after garbage collection), > correspondences between the machine program counter and source locations for stack walking. > > This patch: > 1. Add more source positions info in reloc info to make it suitable for source level mapping. > The amount of data should not be increased dramatically because (1) V8 already marks interesting places in the generated code and > (2) V8 does not write redundant information (it writes a pair (pc_offset, pos) only if pos is changed and skips other). > I measured it on Octane benchmark - for unoptimized code the number of source positions may achieve 2x ('lin_solve' from NavierStokes benchmark). > > 2. When a sample happens, CPU profiler finds a code object by pc, then use its reloc info to match the sample to a source line. > If a source line is found that hit counter is increased by one for this line. > > 3. Add a new public V8 API to get the hit source lines by CDT CPU profiler. > Note that it's expected a minor patch in Blink to pack the source level info in JSON to be shown. > > 4.Add a test that checks how the samples are distributed through source lines. > It tests two cases: (1) relocation info created during code generation and (2) relocation info associated with precompiled function's version. > > Patch from Denis Pravdin <denis.pravdin@intel.com>; > BUG=None > LOG=Y > R=svenpanne@chromium.org > > Committed: https://code.google.com/p/v8/source/detail?r=24389 TBR=svenpanne@chromium.org,danno@chromium.org,alph@chromium.org,denis.pravdin@intel.com,weiliang.lin@intel.com BUG=None LOG=N Committed: https://code.google.com/p/v8/source/detail?r=24394

Patch Set 1 #

Patch Set 2 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+55 lines, -346 lines) Patch
M include/v8-profiler.h View 1 2 chunks +0 lines, -20 lines 0 comments Download
M src/api.cc View 1 1 chunk +0 lines, -13 lines 0 comments Download
M src/cpu-profiler.cc View 1 6 chunks +12 lines, -34 lines 0 comments Download
M src/full-codegen.cc View 1 1 chunk +0 lines, -1 line 0 comments Download
M src/profile-generator.h View 1 10 chunks +11 lines, -48 lines 0 comments Download
M src/profile-generator.cc View 1 14 chunks +23 lines, -120 lines 0 comments Download
M src/profile-generator-inl.h View 1 3 chunks +9 lines, -12 lines 0 comments Download
M test/cctest/test-cpu-profiler.cc View 1 1 chunk +0 lines, -98 lines 0 comments Download

Messages

Total messages: 5 (1 generated)
yurys
Created Revert of Extend CPU profiler with mapping ticks to source lines
6 years, 2 months ago (2014-10-02 11:52:37 UTC) #1
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/624443005/1
6 years, 2 months ago (2014-10-02 11:52:53 UTC) #2
commit-bot: I haz the power
No LGTM from a valid reviewer yet. Only full committers are accepted. Even if an ...
6 years, 2 months ago (2014-10-02 11:52:54 UTC) #4
yurys
6 years, 2 months ago (2014-10-02 11:58:33 UTC) #5
Message was sent while issue was closed.
Committed patchset #2 (id:20004) manually as 24394 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698