lgtm

But please refactor the test to avoid using sleep in a separate CL.


http://codereview.chromium.org/9295014/diff/1/test/cctest/test-debug.cc
File test/cctest/test-debug.cc (right):

http://codereview.chromium.org/9295014/diff/1/test/cctest/test-debug.cc#newcode7288
test/cctest/test-debug.cc:7288:
I think this test could have been written in JavaScript using the native
function %OptimizeFunctionOnNextCall and the JavaScript debugger API.
See mjsunit/debug-break-inline.js. Maybe extend
mjsunit/debug-break-inline.js with line number checking as well.

http://codereview.chromium.org/9295014/diff/1/test/cctest/test-debug.cc#newcode7293
test/cctest/test-debug.cc:7293: // Wait for crankshaft to inline f()
into g();
Having tests with sleeps in then is not a good idea, as they will delay
the running of the tests. Please change the test to first run the code
for some iterations then use %OptimizeFunctionOnNextCall (or make
Runtime_OptimizeFunctionOnNextCall available from C tests) to
optimize/inline it. You can pass the number of iterations to f and have
it be relatively small for the warmup and larger for when you want to
use DebugBread from the other thread.

http://codereview.chromium.org/9295014/

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

Reply via email to