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

Issue 18620002: Do not store fp and sp values in TickSample (Closed)

Created:
7 years, 5 months ago by yurys
Modified:
7 years, 5 months ago
Reviewers:
loislo, alph, Jakob Kummerow
CC:
v8-dev
Visibility:
Public.

Description

Do not store fp and sp values in TickSample Their values are not used neither by the tick processor nor by CpuProfiler so it is just a waste of space. TickSample used to be a transport for grabbed register values to TickSample::Trace, now they are passed in a special structure RegisterState which is allocated on the stack for the sampling period. Some common pieces were moved from platform-dependent code into Sampler::SampleStack and TickSample::Init. BUG=None R=jkummerow@chromium.org, loislo@chromium.org Committed: https://code.google.com/p/v8/source/detail?r=15484

Patch Set 1 #

Patch Set 2 : Removed fp #

Total comments: 2

Patch Set 3 : Fixed style as suggested #

Patch Set 4 : Fixed SIMULATOR compilation #

Unified diffs Side-by-side diffs Delta from patch set Stats (+133 lines, -133 lines) Patch
M src/frames.h View 1 1 chunk +1 line, -1 line 0 comments Download
M src/frames.cc View 1 2 1 chunk +5 lines, -4 lines 0 comments Download
M src/log.cc View 1 2 3 1 chunk +0 lines, -2 lines 0 comments Download
M src/sampler.h View 1 2 3 3 chunks +14 lines, -10 lines 0 comments Download
M src/sampler.cc View 1 2 3 8 chunks +91 lines, -95 lines 0 comments Download
M test/cctest/test-log-stack-tracer.cc View 1 2 3 2 chunks +5 lines, -3 lines 0 comments Download
M test/mjsunit/tools/tickprocessor-test.log View 1 chunk +13 lines, -13 lines 0 comments Download
M test/mjsunit/tools/tickprocessor-test-func-info.log View 1 chunk +3 lines, -3 lines 0 comments Download
M tools/tickprocessor.js View 2 chunks +1 line, -2 lines 0 comments Download

Messages

Total messages: 5 (0 generated)
yurys
7 years, 5 months ago (2013-07-03 08:54:30 UTC) #1
loislo
lgtm
7 years, 5 months ago (2013-07-03 09:06:12 UTC) #2
Jakob Kummerow
lgtm https://codereview.chromium.org/18620002/diff/2001/src/frames.cc File src/frames.cc (right): https://codereview.chromium.org/18620002/diff/2001/src/frames.cc#newcode220 src/frames.cc:220: StackFrameIteratorBase(isolate, false), nit: while you're here, please move ...
7 years, 5 months ago (2013-07-03 14:12:31 UTC) #3
yurys
https://codereview.chromium.org/18620002/diff/2001/src/frames.cc File src/frames.cc (right): https://codereview.chromium.org/18620002/diff/2001/src/frames.cc#newcode220 src/frames.cc:220: StackFrameIteratorBase(isolate, false), On 2013/07/03 14:12:31, Jakob wrote: > nit: ...
7 years, 5 months ago (2013-07-03 14:16:41 UTC) #4
yurys
7 years, 5 months ago (2013-07-03 16:21:10 UTC) #5
Message was sent while issue was closed.
Committed patchset #4 manually as r15484 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698