Sorry for long delay. lgtm.
Please make sure you get one from yurys@
Thank you!
https://codereview.chromium.org/424973004/diff/120001/src/cpu-profiler.cc
File src/cpu-profiler.cc (right):
https://codereview.chromium.org/424973004/diff/120001/src/cpu-profiler.cc#newcode261
src/cpu-profiler.cc:261: if (shared->script()->IsScript()) {
On 2014/08/23 05:18:52, Denis Pravdin wrote:
On 2014/08/15 12:10:47, alph wrote:
> Is it possible the shared->script() is not a Script? I see
previously it was
not
> the case.
I reused the same pattern as
CpuProfiler::CodeCreateEvent(Logger::LogEventsAndTags tag, Code* code,
SharedFunctionInfo* shared, CompilationInfo* info, Name* script_name)
has.
It does the same check. Is it OK?
That function is called from a different place, so it may have different
assumptions about script().
This function does always have script() pointing to the correct Script
object, so the check is not necessary.
https://codereview.chromium.org/424973004/diff/120001/src/profile-generator.h
File src/profile-generator.h (right):
https://codereview.chromium.org/424973004/diff/120001/src/profile-generator.h#newcode58
src/profile-generator.h:58:
pc_offset_map_.insert(std::make_pair(pc_offset, line));
I meant:
if (GetSourceLineNumber(pc_offset) != line) {
pc_offset_map_.insert(std::make_pair(pc_offset, line));
}
https://codereview.chromium.org/424973004/diff/180001/src/profile-generator.cc
File src/profile-generator.cc (right):
https://codereview.chromium.org/424973004/diff/180001/src/profile-generator.cc#newcode219
src/profile-generator.cc:219:
line_ticks_.Lookup(reinterpret_cast<void*>(src_line), src_line, true);
bad padding. Could you please run it through 'git cl format'
https://codereview.chromium.org/424973004/diff/180001/src/profile-generator.cc#newcode231
src/profile-generator.cc:231: if (line_count == 0) return false;
it should return true, as it copied all the lines.
https://codereview.chromium.org/424973004/diff/180001/src/profile-generator.h
File src/profile-generator.h (right):
https://codereview.chromium.org/424973004/diff/180001/src/profile-generator.h#newcode69
src/profile-generator.h:69: bool Empty() const { return
pc_offset_map_.empty(); }
style: s/Empty/empty/
https://codereview.chromium.org/424973004/diff/180001/test/cctest/test-cpu-profiler.cc
File test/cctest/test-cpu-profiler.cc (right):
https://codereview.chromium.org/424973004/diff/180001/test/cctest/test-cpu-profiler.cc#newcode1079
test/cctest/test-cpu-profiler.cc:1079: "function %s() {\n"
Does the test function need to be that complex? There seems to be no
need to run loops in it. Isn't it?
https://codereview.chromium.org/424973004/
--
--
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.
