Create code and instruction object, and install them in the background compilation thread while br…

Create code and instruction object, and install them in the background compilation thread while bringing mutator thread to a saferpoint.

BUG=

[srdjan, 2015-11-17 23:04:27]

Description was changed from

==========
Create code and instruction object, and install them in the background
compilatiomn thread while bringing mutator thread to a saferpoint.

BUG=
==========

to

==========
Create code and instruction object, and install them in the background
compilation thread while bringing mutator thread to a saferpoint.

BUG=
==========

[srdjan, 2015-11-17 23:22:24]

srdjan@google.com changed reviewers:
+ asiva@google.com

[siva, 2015-11-18 17:46:27]

lgtm

https://codereview.chromium.org/1454553004/diff/60001/runtime/vm/compiler.cc
File runtime/vm/compiler.cc (right):

https://codereview.chromium.org/1454553004/diff/60001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:407: // if the code may got invalidated while compiling.
The comment does not read well, maybe

Get current generation count so that we can check and ensure that the code was
not invalidated while we were compiling in the background.

https://codereview.chromium.org/1454553004/diff/60001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:750: // Stop mutator thread while in background
compilation.
Stop mutator thread before creating the instruction object.

https://codereview.chromium.org/1454553004/diff/60001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:797: function.InstallOptimizedCode(code, is_osr);
Maybe add a comment here that there won't be concurrent access to the
instruction page as the background compiler installs code only at a safepoint.

https://codereview.chromium.org/1454553004/diff/60001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:799: // Background compilation
Background compilation.

https://codereview.chromium.org/1454553004/diff/60001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/1454553004/diff/60001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:2894:
Isolate::Current()->thread_registry()->InSafepoint());
I wonder if it will be more efficient to do:
#if defined(DEBUG)
Thread* thread = Thread;;Current();
ASSERT(thread->IsMutatorThread() ||
thread->isolate()->thread_registry()->AtSafepoint());
ASSERT(code.is_optimized);
#endif

[srdjan, 2015-11-18 19:04:53]

https://codereview.chromium.org/1454553004/diff/60001/runtime/vm/compiler.cc
File runtime/vm/compiler.cc (right):

https://codereview.chromium.org/1454553004/diff/60001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:407: // if the code may got invalidated while compiling.
On 2015/11/18 17:46:27, siva wrote:
> The comment does not read well, maybe
> 
> Get current generation count so that we can check and ensure that the code was
> not invalidated while we were compiling in the background.

Done.

https://codereview.chromium.org/1454553004/diff/60001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:750: // Stop mutator thread while in background
compilation.
On 2015/11/18 17:46:27, siva wrote:
> Stop mutator thread before creating the instruction object.

Stop mutator thread before creating the instruction object and installing code.

https://codereview.chromium.org/1454553004/diff/60001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:797: function.InstallOptimizedCode(code, is_osr);
On 2015/11/18 17:46:27, siva wrote:
> Maybe add a comment here that there won't be concurrent access to the
> instruction page as the background compiler installs code only at a safepoint.


Adding that comment above at FinalizeCode:
        // Allocates instruction object. Since this occurs only at safepoint,
        // there can be no concurrent access to the instruction page.

Adding here:
           // Install code while at safepoint.

https://codereview.chromium.org/1454553004/diff/60001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:799: // Background compilation
On 2015/11/18 17:46:27, siva wrote:
> Background compilation.
> 

Done and added:
            // Before installing code check generation counts if the code may
            // have become invalid.

https://codereview.chromium.org/1454553004/diff/60001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/1454553004/diff/60001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:2894:
Isolate::Current()->thread_registry()->InSafepoint());
On 2015/11/18 17:46:27, siva wrote:
> I wonder if it will be more efficient to do:
> #if defined(DEBUG)
> Thread* thread = Thread;;Current();
> ASSERT(thread->IsMutatorThread() ||
> thread->isolate()->thread_registry()->AtSafepoint());
> ASSERT(code.is_optimized);
> #endif

Factoring this out into (in object.cc):

static bool IsMutatorOrAtSafepoint() {...}

Also changing ThreadRegistry's InSafepoint -> AtSafepoint