This adds a mechanism to invalidate code that was generated in the background. The invalidation can…

This adds a mechanism to invalidate code that was generated in the background. The invalidation can happen if a relevant event was invoked between background compilation start and installation of code (e.g, field type invalidatet, subclasses added, deferred loading).

We introduce 3 invalidation generation counters: CHA, field and library prefixes. If one of the counters was incremented during compilation, and is relevant for that compilation, the code installation is skipped since that code may have become invalid in the meantime.

Add BackgroundCompilationResult that remembers the 3 invalidation generations.

The generational counter can rollover, which is OK.

BUG=
R=asiva@google.com

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

[srdjan, 2015-10-28 22:19:59]

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

[srdjan, 2015-10-29 20:41:56]

Description was changed from

==========
This adds a mechanism to invalidate code that was generated in the background.
The invalidation can happen if a relevant event was invoked between background
compilation start and installation of code (e.g, field type invalidatet,
subclasses added, deferred loading).

We introduce 3 invalidation generation counters: CHA, field and library
prefixes. If one of the counters was incremented during compilation, and is
relevant for that compilation, the code installation is skipped since that code
may have become invalid in the meantime.

Add BackgroundCompilationResult that remembers the 3 invalidation generations.

TODO: reset invalidation generation counters when background compiler is idle
and queues are empty.

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

to

==========
This adds a mechanism to invalidate code that was generated in the background.
The invalidation can happen if a relevant event was invoked between background
compilation start and installation of code (e.g, field type invalidatet,
subclasses added, deferred loading).

We introduce 3 invalidation generation counters: CHA, field and library
prefixes. If one of the counters was incremented during compilation, and is
relevant for that compilation, the code installation is skipped since that code
may have become invalid in the meantime.

Add BackgroundCompilationResult that remembers the 3 invalidation generations.

The generational counter can rollover, which is OK. They must fix into a Smi
though.


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

[srdjan, 2015-10-29 21:48:30]

Description was changed from

==========
This adds a mechanism to invalidate code that was generated in the background.
The invalidation can happen if a relevant event was invoked between background
compilation start and installation of code (e.g, field type invalidatet,
subclasses added, deferred loading).

We introduce 3 invalidation generation counters: CHA, field and library
prefixes. If one of the counters was incremented during compilation, and is
relevant for that compilation, the code installation is skipped since that code
may have become invalid in the meantime.

Add BackgroundCompilationResult that remembers the 3 invalidation generations.

The generational counter can rollover, which is OK. They must fix into a Smi
though.


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

to

==========
This adds a mechanism to invalidate code that was generated in the background.
The invalidation can happen if a relevant event was invoked between background
compilation start and installation of code (e.g, field type invalidatet,
subclasses added, deferred loading).

We introduce 3 invalidation generation counters: CHA, field and library
prefixes. If one of the counters was incremented during compilation, and is
relevant for that compilation, the code installation is skipped since that code
may have become invalid in the meantime.

Add BackgroundCompilationResult that remembers the 3 invalidation generations.

The generational counter can rollover, which is OK. 

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

[siva, 2015-10-29 23:42:08]

lgtm

https://codereview.chromium.org/1418813008/diff/40001/runtime/vm/compiler.h
File runtime/vm/compiler.h (right):

https://codereview.chromium.org/1418813008/diff/40001/runtime/vm/compiler.h#n...
runtime/vm/compiler.h:30: // Initializes with current isolate-stored
invalidation generations
generations.

https://codereview.chromium.org/1418813008/diff/60001/runtime/vm/isolate.h
File runtime/vm/isolate.h (right):

https://codereview.chromium.org/1418813008/diff/60001/runtime/vm/isolate.h#ne...
runtime/vm/isolate.h:857: intptr_t prefix_invalidation_gen_;
why not make these uint64_t instead of intptr_t values.

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

https://codereview.chromium.org/1418813008/diff/80001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:1599: CompilationWorkQueue* queue) const {
should assert that this thread owns the lock for queue?

https://codereview.chromium.org/1418813008/diff/80001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:1607: void
BackgroundCompilationResult::PopFromQueue(CompilationWorkQueue* queue) {
should assert that this thread owns the lock for queue?

https://codereview.chromium.org/1418813008/diff/80001/runtime/vm/compiler.h
File runtime/vm/compiler.h (right):

https://codereview.chromium.org/1418813008/diff/80001/runtime/vm/compiler.h#n...
runtime/vm/compiler.h:44: cha_invalidation_gen_ = Integer::null();
Instead of clearing these to null, can you clear them to some sentinel integer
value, that way PopBackInteger does not need that special check for null and the
associated handle creation.

https://codereview.chromium.org/1418813008/diff/80001/runtime/vm/compiler.h#n...
runtime/vm/compiler.h:56: void PrintValidity();
void PrintValidity() const;

https://codereview.chromium.org/1418813008/diff/80001/runtime/vm/compiler.h#n...
runtime/vm/compiler.h:193: Monitor* queue_monitor_;  // Controls access to the
queue.
I am wondering if it makes sense to have this monitor live adjacent to the queue
it protects.

[srdjan, 2015-10-30 17:21:12]

https://codereview.chromium.org/1418813008/diff/40001/runtime/vm/compiler.h
File runtime/vm/compiler.h (right):

https://codereview.chromium.org/1418813008/diff/40001/runtime/vm/compiler.h#n...
runtime/vm/compiler.h:30: // Initializes with current isolate-stored
invalidation generations
On 2015/10/29 23:42:07, siva wrote:
> generations.

Done.

https://codereview.chromium.org/1418813008/diff/60001/runtime/vm/isolate.h
File runtime/vm/isolate.h (right):

https://codereview.chromium.org/1418813008/diff/60001/runtime/vm/isolate.h#ne...
runtime/vm/isolate.h:857: intptr_t prefix_invalidation_gen_;
On 2015/10/29 23:42:07, siva wrote:
> why not make these uint64_t instead of intptr_t values.

Making them uint32_t, since we already have Integer::AsTruncatedUint32Value().
This counters could overflow, and that is OK, added comment.

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

https://codereview.chromium.org/1418813008/diff/80001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:1599: CompilationWorkQueue* queue) const {
On 2015/10/29 23:42:07, siva wrote:
> should assert that this thread owns the lock for queue?

Discussed offline: in a followup CL will move the queue from isolate into
BackgroundCompiler, so that it is guaranteed that the queue access is
synchronized.

https://codereview.chromium.org/1418813008/diff/80001/runtime/vm/compiler.cc#...
runtime/vm/compiler.cc:1607: void
BackgroundCompilationResult::PopFromQueue(CompilationWorkQueue* queue) {
On 2015/10/29 23:42:07, siva wrote:
> should assert that this thread owns the lock for queue?

Ditto

https://codereview.chromium.org/1418813008/diff/80001/runtime/vm/compiler.h
File runtime/vm/compiler.h (right):

https://codereview.chromium.org/1418813008/diff/80001/runtime/vm/compiler.h#n...
runtime/vm/compiler.h:44: cha_invalidation_gen_ = Integer::null();
On 2015/10/29 23:42:07, siva wrote:
> Instead of clearing these to null, can you clear them to some sentinel integer
> value, that way PopBackInteger does not need that special check for null and
the
> associated handle creation.

Discussed offline and leaving it as it is. A sentinel would require extra checks
in increment of generation and this code is not high performance code (occur 3
times per compilation).

https://codereview.chromium.org/1418813008/diff/80001/runtime/vm/compiler.h#n...
runtime/vm/compiler.h:56: void PrintValidity();
On 2015/10/29 23:42:08, siva wrote:
> void PrintValidity() const;

Done.

https://codereview.chromium.org/1418813008/diff/80001/runtime/vm/compiler.h#n...
runtime/vm/compiler.h:193: Monitor* queue_monitor_;  // Controls access to the
queue.
On 2015/10/29 23:42:08, siva wrote:
> I am wondering if it makes sense to have this monitor live adjacent to the
queue
> it protects.

Yes, next CL.

[srdjan, 2015-10-30 19:19:06]

Committed patchset #7 (id:120001) manually as
fe4a5d7672b878555b357005273877c8d71e91f0 (presubmit successful).