Make ICData changes thread safe (first compute array, then set it). Install code in the main thread…

Make ICData changes thread safe (first compute array, then set it). Install code in the main thread instead of in the background compilation thread.

BUG=
R=asiva@google.com

Committed: https://github.com/dart-lang/sdk/commit/772ee329c4a5319b6a8a1ea15e7fb0999f03fdfd

[srdjan, 2015-10-26 18:07:46]

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

[siva, 2015-10-26 21:18:25]

lgtm

https://chromiumcodereview.appspot.com/1410363005/diff/60001/runtime/vm/compi...
File runtime/vm/compiler.cc (right):

https://chromiumcodereview.appspot.com/1410363005/diff/60001/runtime/vm/compi...
runtime/vm/compiler.cc:1442: // Use PushFront and PopBack to add and read from
queue.
PushFrontFunction and PopBackFunction to add and remove from the function queue
and PushBackCode and PopBackCode to add and remove from the code queue.

https://chromiumcodereview.appspot.com/1410363005/diff/60001/runtime/vm/compi...
runtime/vm/compiler.cc:1641: task = NULL;
What is the purpose of setting task to NULL it is not used in this function and
is not visible to the caller?

https://chromiumcodereview.appspot.com/1410363005/diff/60001/runtime/vm/compi...
File runtime/vm/compiler.h (right):

https://chromiumcodereview.appspot.com/1410363005/diff/60001/runtime/vm/compi...
runtime/vm/compiler.h:113: // No OSR compilation the background compiler.
No OSR compilation in the background compiler.

https://chromiumcodereview.appspot.com/1410363005/diff/60001/runtime/vm/compi...
runtime/vm/compiler.h:127: // Access to queue length is guarded with
queue_monitor_;
(maybe once we have the functionality to assert queue_monitor_ is owned we can
assert here that queue_monitor_ is held by current thread).

https://chromiumcodereview.appspot.com/1410363005/diff/60001/runtime/vm/objec...
File runtime/vm/object.cc (right):

https://chromiumcodereview.appspot.com/1410363005/diff/60001/runtime/vm/objec...
runtime/vm/object.cc:5395:
ASSERT(Isolate::Current()->MutatorThreadIsCurrentThread());
when syncing to top of tree you would have to change this to
ASSERT(Thread::Current()->IsMutatorThread());

[srdjan, 2015-10-26 21:57:00]

https://chromiumcodereview.appspot.com/1410363005/diff/60001/runtime/vm/compi...
File runtime/vm/compiler.cc (right):

https://chromiumcodereview.appspot.com/1410363005/diff/60001/runtime/vm/compi...
runtime/vm/compiler.cc:1442: // Use PushFront and PopBack to add and read from
queue.
On 2015/10/26 21:18:25, siva wrote:
> PushFrontFunction and PopBackFunction to add and remove from the function
queue
> and PushBackCode and PopBackCode to add and remove from the code queue.

Done.

https://chromiumcodereview.appspot.com/1410363005/diff/60001/runtime/vm/compi...
runtime/vm/compiler.cc:1641: task = NULL;
On 2015/10/26 21:18:25, siva wrote:
> What is the purpose of setting task to NULL it is not used in this function
and
> is not visible to the caller?

It is to prevent accessing task after we set task->running_ to false as it will
be deleted by the thread pool.

https://chromiumcodereview.appspot.com/1410363005/diff/60001/runtime/vm/compi...
File runtime/vm/compiler.h (right):

https://chromiumcodereview.appspot.com/1410363005/diff/60001/runtime/vm/compi...
runtime/vm/compiler.h:113: // No OSR compilation the background compiler.
On 2015/10/26 21:18:25, siva wrote:
> No OSR compilation in the background compiler.

Done.

https://chromiumcodereview.appspot.com/1410363005/diff/60001/runtime/vm/compi...
runtime/vm/compiler.h:127: // Access to queue length is guarded with
queue_monitor_;
On 2015/10/26 21:18:25, siva wrote:
> (maybe once we have the functionality to assert queue_monitor_ is owned we can
> assert here that queue_monitor_ is held by current thread).

Acknowledged.

https://chromiumcodereview.appspot.com/1410363005/diff/60001/runtime/vm/objec...
File runtime/vm/object.cc (right):

https://chromiumcodereview.appspot.com/1410363005/diff/60001/runtime/vm/objec...
runtime/vm/object.cc:5395:
ASSERT(Isolate::Current()->MutatorThreadIsCurrentThread());
On 2015/10/26 21:18:25, siva wrote:
> when syncing to top of tree you would have to change this to
> ASSERT(Thread::Current()->IsMutatorThread());

Done.

[srdjan, 2015-10-26 22:12:40]

Committed patchset #5 (id:80001) manually as
772ee329c4a5319b6a8a1ea15e7fb0999f03fdfd (presubmit successful).