Parallel recompilation: remove interrupt for code generation.

Parallel recompilation: remove interrupt for code generation.


R=jkummerow@chromium.org
BUG=

Committed: https://code.google.com/p/v8/source/detail?r=13917

[Jakob Kummerow, 2013-03-12 15:19:30]

LGTM with comments.

https://chromiumcodereview.appspot.com/12488006/diff/1/src/execution.cc
File src/execution.cc (right):

https://chromiumcodereview.appspot.com/12488006/diff/1/src/execution.cc#newco...
src/execution.cc:882: return isolate->heap()->undefined_value();
Probably better to do this change of behavior in a separate CL to isolate any
potential performance impact.

https://chromiumcodereview.appspot.com/12488006/diff/1/src/objects.cc
File src/objects.cc (right):

https://chromiumcodereview.appspot.com/12488006/diff/1/src/objects.cc#newcode...
src/objects.cc:8003: // No write barrier required, since the builtin is part of
the root set.
Since this is true for all builtins, please update ReplaceCode(...builtin...)
calls in the vicinity for consistency.

https://chromiumcodereview.appspot.com/12488006/diff/1/src/optimizing-compile...
File src/optimizing-compiler-thread.cc (right):

https://chromiumcodereview.appspot.com/12488006/diff/1/src/optimizing-compile...
src/optimizing-compiler-thread.cc:93: // After putting the finished function
onto the install queue but before
This comment is misleading. Maybe something like:
The execution thread can call InstallOptimizedFunctions() at any time, including
right now, but that function doesn't do anything with the optimizing_compiler we
just enqueued unless its closure is marked for installing recompiled code.

[Sven Panne, 2013-03-12 15:44:51]

Some DBCs...

https://chromiumcodereview.appspot.com/12488006/diff/1/src/execution.cc
File src/execution.cc (right):

https://chromiumcodereview.appspot.com/12488006/diff/1/src/execution.cc#newco...
src/execution.cc:882: return isolate->heap()->undefined_value();
On 2013/03/12 15:19:30, Jakob wrote:
> Probably better to do this change of behavior in a separate CL to isolate any
> potential performance impact.

+1 for a separate CL, I considered something similar in the past, and there was
some impact, but I can't remember the details. :-)

It would be very good if we could return here, because this would remove some
flakiness.

Hmmm, thinking about it: Is it really still the case than that interrupt flags
form a *set*? Isn't there only a single reason for any interupt? Not sure, but
it would be worthwhile to find out...

https://chromiumcodereview.appspot.com/12488006/diff/1/src/runtime.cc
File src/runtime.cc (right):

https://chromiumcodereview.appspot.com/12488006/diff/1/src/runtime.cc#newcode...
src/runtime.cc:7752: CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 0);
Can we please move the argument access macros (and any related HandleScope) to
the beginning of the runtime functions? This way error checking does not depend
on e.g. the value of any flags or other random circumstances.

https://chromiumcodereview.appspot.com/12488006/diff/1/src/runtime.cc#newcode...
src/runtime.cc:7928: CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 0);
Same as above.

[Yang, 2013-03-12 18:03:34]

Committed patchset #2 manually as r13917 (presubmit successful).

[Yang, 2013-03-12 18:03:38]

https://chromiumcodereview.appspot.com/12488006/diff/1/src/execution.cc
File src/execution.cc (right):

https://chromiumcodereview.appspot.com/12488006/diff/1/src/execution.cc#newco...
src/execution.cc:882: return isolate->heap()->undefined_value();
On 2013/03/12 15:44:51, Sven Panne wrote:
> On 2013/03/12 15:19:30, Jakob wrote:
> > Probably better to do this change of behavior in a separate CL to isolate
any
> > potential performance impact.
> 
> +1 for a separate CL, I considered something similar in the past, and there
was
> some impact, but I can't remember the details. :-)
> 
> It would be very good if we could return here, because this would remove some
> flakiness.
> 
> Hmmm, thinking about it: Is it really still the case than that interrupt flags
> form a *set*? Isn't there only a single reason for any interupt? Not sure, but
> it would be worthwhile to find out...

Done.

https://chromiumcodereview.appspot.com/12488006/diff/1/src/objects.cc
File src/objects.cc (right):

https://chromiumcodereview.appspot.com/12488006/diff/1/src/objects.cc#newcode...
src/objects.cc:8003: // No write barrier required, since the builtin is part of
the root set.
On 2013/03/12 15:19:30, Jakob wrote:
> Since this is true for all builtins, please update ReplaceCode(...builtin...)
> calls in the vicinity for consistency.

Done.

https://chromiumcodereview.appspot.com/12488006/diff/1/src/optimizing-compile...
File src/optimizing-compiler-thread.cc (right):

https://chromiumcodereview.appspot.com/12488006/diff/1/src/optimizing-compile...
src/optimizing-compiler-thread.cc:93: // After putting the finished function
onto the install queue but before
On 2013/03/12 15:19:30, Jakob wrote:
> This comment is misleading. Maybe something like:
> The execution thread can call InstallOptimizedFunctions() at any time,
including
> right now, but that function doesn't do anything with the optimizing_compiler
we
> just enqueued unless its closure is marked for installing recompiled code.

Done.

https://chromiumcodereview.appspot.com/12488006/diff/1/src/runtime.cc
File src/runtime.cc (right):

https://chromiumcodereview.appspot.com/12488006/diff/1/src/runtime.cc#newcode...
src/runtime.cc:7752: CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 0);
On 2013/03/12 15:44:51, Sven Panne wrote:
> Can we please move the argument access macros (and any related HandleScope) to
> the beginning of the runtime functions? This way error checking does not
depend
> on e.g. the value of any flags or other random circumstances.

Done.

https://chromiumcodereview.appspot.com/12488006/diff/1/src/runtime.cc#newcode...
src/runtime.cc:7928: CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 0);
On 2013/03/12 15:44:51, Sven Panne wrote:
> Same as above.

Done.