ptal

https://codereview.chromium.org/596533002/diff/80001/src/vm-state.h
File src/vm-state.h (right):

https://codereview.chromium.org/596533002/diff/80001/src/vm-state.h#newcode19
src/vm-state.h:19:
On 2014/09/23 14:57:53, yurys wrote:
style: remove empty line.

Done.

https://codereview.chromium.org/596533002/diff/80001/test/cctest/test-sampler-api.cc
File test/cctest/test-sampler-api.cc (right):

https://codereview.chromium.org/596533002/diff/80001/test/cctest/test-sampler-api.cc#newcode22
test/cctest/test-sampler-api.cc:22: using v8::Local;
On 2014/09/23 14:57:53, yurys wrote:
style nit: we don't usually declare such using in other places in v8,
so I'd
rather be consistent with the rest of the codebase.

Done.

https://codereview.chromium.org/596533002/diff/80001/test/cctest/test-sampler-api.cc#newcode83
test/cctest/test-sampler-api.cc:83: #define
SAMPLER_API_TESTS_BOOTSTRAP()                                       \
On 2014/09/23 14:57:53, yurys wrote:
style: I'd use a class rather than a macro here.

Done.

https://codereview.chromium.org/596533002/diff/80001/test/cctest/test-sampler-api.cc#newcode112
test/cctest/test-sampler-api.cc:112: int MAX_SIZE =
Sample::kFramesLimit;
On 2014/09/23 14:57:53, yurys wrote:
inline this

Done.

https://codereview.chromium.org/596533002/diff/80001/test/cctest/test-sampler-api.cc#newcode118
test/cctest/test-sampler-api.cc:118: std::vector<v8::JitCodeEvent>
inner_funcs;
On 2014/09/23 14:57:53, yurys wrote:
JitCodeEvent contains Handle<Script>, is it a global or local handle?
In the
latter case it may be unsafe to put it into a global vector like this
as
corresponding HandleScope may be destroyed before the handle.

Done.

https://codereview.chromium.org/596533002/diff/80001/test/cctest/test-sampler-api.cc#newcode122
test/cctest/test-sampler-api.cc:122: if (event->type !=
v8::JitCodeEvent::CODE_ADDED) return;
On 2014/09/23 14:57:53, yurys wrote:
What about v8::JitCodeEvent::CODE_MOVE? How can we be sure that GC
won't move
the code during the test?

Done.

https://codereview.chromium.org/596533002/diff/80001/test/cctest/test-sampler-api.cc#newcode146
test/cctest/test-sampler-api.cc:146: // They should fall in the range
where the compiled code
On 2014/09/23 14:57:53, yurys wrote:
typo: ... where the compiled code resides.

Done.

https://codereview.chromium.org/596533002/diff/80001/test/cctest/test-sampler-api.cc#newcode166
test/cctest/test-sampler-api.cc:166: (int64_t)inner_funcs[i].code_start
+ inner_funcs[i].code_len);
On 2014/09/23 14:57:53, yurys wrote:
static_cast<intptr_t>(inner_funcs[i].code_start) as we don't use
c-style cast.

Done.

https://codereview.chromium.org/596533002/diff/80001/test/cctest/test-sampler-api.cc#newcode188
test/cctest/test-sampler-api.cc:188: class SimulatorHelper {
On 2014/09/23 14:57:53, yurys wrote:
I don't think we need to pull all these platform-specific code here.
Since we
always call GetSample after exiting into the runtime we can pass
address of some
C-function, fp and sp values as the register state, they should be
ignored by
the stack iterator as it should take top frame from
Isolate::c_entry_fp().

Done.

https://codereview.chromium.org/596533002/

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/d/optout.

Reply via email to