https://codereview.chromium.org/424973004/diff/40001/src/cpu-profiler.cc
File src/cpu-profiler.cc (right):

https://codereview.chromium.org/424973004/diff/40001/src/cpu-profiler.cc#newcode260
src/cpu-profiler.cc:260: ASSERT(Script::cast(shared->script()));
nit: DCHECK here and everywhere else.

Also cast does SLOW_DCHECK internally.
Anyway if you still want to do the check here, it's better to use:
DCHECK(shared->script()->IsScript());

https://codereview.chromium.org/424973004/diff/40001/src/cpu-profiler.cc#newcode265
src/cpu-profiler.cc:265: Handle<Script>
script_handle(Script::cast(shared->script()));
nit: you can use the 'script' pointer calculated above.

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

https://codereview.chromium.org/424973004/diff/40001/src/profile-generator.cc#newcode226
src/profile-generator.cc:226: e->value = static_cast<char*>(e->value) +
1;
should be cast to uintptr_t

https://codereview.chromium.org/424973004/diff/40001/src/profile-generator.cc#newcode244
src/profile-generator.cc:244: entry->line =
reinterpret_cast<uintptr_t>(p->key);
I still bet it won't compile on x64 because of implicit 64->32 bits
conversion. You have to write something like:
static_cast<usigned int>(reinterpret_cast<uintptr_t>(p->key));

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

https://codereview.chromium.org/424973004/diff/40001/test/cctest/test-cpu-profiler.cc#newcode1093
test/cctest/test-cpu-profiler.cc:1093: if (strcmp(str.get(), name) == 0)
It's better to make a v8 heap string out of 'name' using
v8::String::NewFromUtf8 so you can compare two heap strings instead.
This way you won't need to flatten each string in the heap to make the
comparison.

https://codereview.chromium.org/424973004/diff/40001/test/cctest/test-cpu-profiler.cc#newcode1116
test/cctest/test-cpu-profiler.cc:1116: "  while (Date.now() - start <
timeout) {\n"
This test bound to be flaky as it depends on the timeouts.

Instead if you want the foo function surely got into profile, make a
call to startProfiling right out of foo. startProfiling always takes the
first sample at its invocation point. See BoundFunctionCall test for
example.

https://codereview.chromium.org/424973004/diff/40001/test/cctest/test-cpu-profiler.cc#newcode1129
test/cctest/test-cpu-profiler.cc:1129: i::SharedFunctionInfo* shared =
GetFunctionInfoFromHeap(isolate, foo_name);
You can get the JSFunction object out of global context after CompileRun
instead of iterating the whole heap. See CreateCode function body above
for example.

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