https://codereview.chromium.org/424973004/diff/20001/include/v8-profiler.h
File include/v8-profiler.h (right):

https://codereview.chromium.org/424973004/diff/20001/include/v8-profiler.h#newcode25
include/v8-profiler.h:25: typedef struct {
style: we use C++ style for struct declaration.

struct LineTick {
...
}

https://codereview.chromium.org/424973004/diff/20001/include/v8-profiler.h#newcode27
include/v8-profiler.h:27: int line;
How about providing column as well which I already suggested before? On
minified sources line number doesn't give enough information as the
whole script may be formatted as one line.

https://codereview.chromium.org/424973004/diff/20001/src/profile-generator.cc
File src/profile-generator.cc (right):

https://codereview.chromium.org/424973004/diff/20001/src/profile-generator.cc#newcode231
src/profile-generator.cc:231: unsigned int number) const {
nit: number -> length

https://codereview.chromium.org/424973004/diff/20001/src/profile-generator.cc#newcode234
src/profile-generator.cc:234: unsigned line_number =
line_ticks_.occupancy();
line_count

https://codereview.chromium.org/424973004/diff/20001/src/profile-generator.h
File src/profile-generator.h (right):

https://codereview.chromium.org/424973004/diff/20001/src/profile-generator.h#newcode162
src/profile-generator.h:162: unsigned int number) const;
nit:number -> length

https://codereview.chromium.org/424973004/diff/20001/test/cctest/test-cpu-profiler.cc
File test/cctest/test-cpu-profiler.cc (right):

https://codereview.chromium.org/424973004/diff/20001/test/cctest/test-cpu-profiler.cc#newcode1074
test/cctest/test-cpu-profiler.cc:1074: "Logger::LogCompiledFunctions");
"GetFunctionCodeFromHeap"

https://codereview.chromium.org/424973004/diff/20001/test/cctest/test-cpu-profiler.cc#newcode1123
test/cctest/test-cpu-profiler.cc:1123: "})(this)\n", foo_name);
'this' passed as timeout looks like an error.

https://codereview.chromium.org/424973004/diff/20001/test/cctest/test-cpu-profiler.cc#newcode1139
test/cctest/test-cpu-profiler.cc:1139: v8::base::OS::Sleep(100);  //
Ensure that a new node is added to the profile.
We need to use some explicit event instead of Sleep. I believe calling
profiler->StopProfiling below should be enough as it should trigger
processing of all enqueued events.

https://codereview.chromium.org/424973004/diff/20001/test/cctest/test-cpu-profiler.cc#newcode1143
test/cctest/test-cpu-profiler.cc:1143: CodeEntry* foo =
generator->code_map()->FindEntry(address);
This is not thread-safe as code_map is used on the processor thread.

It should be enough to add a few fake events into the queue, stop
profiling and check that resulting tree reflects corresponding hit
counts.

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.

Reply via email to