Re-reland "Flush parallel recompilation queues on context dispose notification"

Re-reland "Flush parallel recompilation queues on context dispose notification"

BUG=
R=hpayer@chromium.org, mstarzinger@chromium.org

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

[Yang, 2013-08-06 09:02:47]

The first patch set is the original CL. The second patch set is the fix on top
of it.

I decided to leave the queue flushing in the main thread since I don't want to
violate some invariants (no handle dereferencing and queue flushing in optimizer
thread). The mechanism for Flush() now mirrors that of Stop().

[Michael Starzinger, 2013-08-06 17:57:56]

https://codereview.chromium.org/22379002/diff/3001/src/objects.h
File src/objects.h (right):

https://codereview.chromium.org/22379002/diff/3001/src/objects.h#newcode1256
src/objects.h:1256: V(kGeneratorFunction, "Generator function")                 
               \
nit: Can we keep this list alpha-sorted?

https://codereview.chromium.org/22379002/diff/3001/src/objects.h#newcode1320
src/objects.h:1320: V(kReplacedByLiveEdit, "Replaced by LiveEdit")
nit: Can we keep this list alpha-sorted?

https://codereview.chromium.org/22379002/diff/3001/src/optimizing-compiler-th...
File src/optimizing-compiler-thread.cc (right):

https://codereview.chromium.org/22379002/diff/3001/src/optimizing-compiler-th...
src/optimizing-compiler-thread.cc:135: Release_Store(&stop_thread_,
static_cast<AtomicWord>(FLUSH));
This is still racey, the FLUSH command might be executed by the compiler thread
before the input_queue_semaphore below has been signaled. Which means the below
signal would hit the new semaphore, not the old one. Consider the following
ordering, where "M" is the main thread and "C" is the compiler thread.

M: stop_thread = FLUSH
C: input_queue_semaphore = new Semaphore
C: input_queue_semaphore->Wait()
M: input_queue_semaphore->Signal()
M: input_queue->flush()
M: stop_thread = CONTINUE
C: CompileNext(), but input_queue is empty

Resetting the input_queue_semaphore is mighty dangerous.

[Hannes Payer (out of office), 2013-08-07 09:02:11]

https://codereview.chromium.org/22379002/diff/22001/src/optimizing-compiler-t...
File src/optimizing-compiler-thread.cc (right):

https://codereview.chromium.org/22379002/diff/22001/src/optimizing-compiler-t...
src/optimizing-compiler-thread.cc:76: FlushOutputQueue(true);
You could flush the output queue in the main thread, since this is the consumer
of the queue. But it is also safe to flush it here.

https://codereview.chromium.org/22379002/diff/22001/src/optimizing-compiler-t...
src/optimizing-compiler-thread.cc:169: // happens in CompileNext on the same
thread.
Can we add a comment that we need that code just for testing purposes.

https://codereview.chromium.org/22379002/diff/22001/src/optimizing-compiler-t...
src/optimizing-compiler-thread.cc:174: FlushOutputQueue(false);
Can we call flush in the main thread before calling stop? Flushing of the output
queue could still be done here.

[Yang, 2013-08-07 09:07:38]

https://codereview.chromium.org/22379002/diff/22001/src/optimizing-compiler-t...
File src/optimizing-compiler-thread.cc (right):

https://codereview.chromium.org/22379002/diff/22001/src/optimizing-compiler-t...
src/optimizing-compiler-thread.cc:76: FlushOutputQueue(true);
On 2013/08/07 09:02:11, Hannes Payer wrote:
> You could flush the output queue in the main thread, since this is the
consumer
> of the queue. But it is also safe to flush it here.

Done.

https://codereview.chromium.org/22379002/diff/22001/src/optimizing-compiler-t...
src/optimizing-compiler-thread.cc:169: // happens in CompileNext on the same
thread.
On 2013/08/07 09:02:11, Hannes Payer wrote:
> Can we add a comment that we need that code just for testing purposes.

Done.

https://codereview.chromium.org/22379002/diff/22001/src/optimizing-compiler-t...
src/optimizing-compiler-thread.cc:174: FlushOutputQueue(false);
On 2013/08/07 09:02:11, Hannes Payer wrote:
> Can we call flush in the main thread before calling stop? Flushing of the
output
> queue could still be done here.

Stop() is only called when the isolate is being torn down. At that point, we
don't care to replace the function's code with the unoptimized code when
aborting optimization. Flushing the output queue would be slightly more
expensive since it does that.

[Michael Starzinger, 2013-08-07 09:11:11]

LGTM (if nits are addressed).

https://codereview.chromium.org/22379002/diff/28001/src/optimizing-compiler-t...
File src/optimizing-compiler-thread.cc (right):

https://codereview.chromium.org/22379002/diff/28001/src/optimizing-compiler-t...
src/optimizing-compiler-thread.cc:74: { AllowHandleDereference
allow_handle_dereference;
nit: Since the AllowHandleDereference scope is only for debugging purposes, can
we move it into FlushInputQueue to simplify this call-site and keep the two
flushing methods in sync?

https://codereview.chromium.org/22379002/diff/28001/src/optimizing-compiler-t...
src/optimizing-compiler-thread.cc:126: input_queue_semaphore_->Wait();
nit: Since this is only safe when being called in the compiler thread, can we
add an ASSERT(IsOptimizerThread()) right here above the wait, below the
comments.

[Hannes Payer (out of office), 2013-08-07 09:17:32]

LGTM! With one comment.

https://codereview.chromium.org/22379002/diff/31001/src/optimizing-compiler-t...
File src/optimizing-compiler-thread.cc (right):

https://codereview.chromium.org/22379002/diff/31001/src/optimizing-compiler-t...
src/optimizing-compiler-thread.cc:77: NoBarrier_Store(&queue_length_,
static_cast<AtomicWord>(0));
I think we need a memory barrier for both stores on ARM. WDYT?

[Yang, 2013-08-07 09:33:21]

Committed patchset #6 manually as r16095 (presubmit successful).