VM: [Kernel] Cherry-pick from dart-lang/kernel_sdk

VM: [Kernel] Cherry-pick from dart-lang/kernel_sdk

Fix a trio of issues in type finalization.

1. We were using is_type_finalized to detect classes that had already been
   initialized by DilReader::ReadPreliminaryClass.  This state is not
   guaranteed by this function, however is_cycle_free is.

2. In the VM, classes that were created type finalized were not marked as
   cycle free because finalization did not consider them so cycle-freeness
   didn't matter.  We would therefore try to initialize these classes with
   DilReader::ReadPreliminaryClass and mutate types in them.  This causes
   them to have unfinalized types but still have is_type_finalized true.

3. Types in top-level procedures were not necessarily finalized.  A simple
   way to achieve this is to finalize each library's top-level class.

Review URL: https://chromereviews.googleplex.com/520437013 .

R=kmillikin@google.com

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

[kustermann, 2016-11-21 14:46:53]

kustermann@google.com changed reviewers:
+ kmillikin@google.com, vegorov@google.com

[Kevin Millikin (Google), 2016-11-21 15:00:11]

LGTM when my comment is addressed.

https://codereview.chromium.org/2514373002/diff/1/runtime/vm/kernel_reader.cc
File runtime/vm/kernel_reader.cc (right):

https://codereview.chromium.org/2514373002/diff/1/runtime/vm/kernel_reader.cc...
runtime/vm/kernel_reader.cc:206: classes.Add(ReadClass(library, kernel_klass),
Heap::kOld);
When bootstrapping, ReadClass is also adding classes to the pending class list. 
I guess that's safe but it does extra work and grows the pending class list too
much.

I think the only purpose of the bootstrapping_ flag was to know if we should
finalize classes more or less eagerly.  It probably isn't needed any more.

[kustermann, 2016-11-21 15:30:05]

https://codereview.chromium.org/2514373002/diff/1/runtime/vm/kernel_reader.cc
File runtime/vm/kernel_reader.cc (right):

https://codereview.chromium.org/2514373002/diff/1/runtime/vm/kernel_reader.cc...
runtime/vm/kernel_reader.cc:206: classes.Add(ReadClass(library, kernel_klass),
Heap::kOld);
On 2016/11/21 15:00:11, Kevin Millikin (Google) wrote:
> When bootstrapping, ReadClass is also adding classes to the pending class
list. 
> I guess that's safe but it does extra work and grows the pending class list
too
> much.
> 
> I think the only purpose of the bootstrapping_ flag was to know if we should
> finalize classes more or less eagerly.  It probably isn't needed any more.

Good point. Done.

[kustermann, 2016-11-21 15:38:37]

Description was changed from

==========
VM: [Kernel] Cherry-pick from dart-lang/kernel_sdk

Fix a trio of issues in type finalization.

1. We were using is_type_finalized to detect classes that had already been
   initialized by DilReader::ReadPreliminaryClass.  This state is not
   guaranteed by this function, however is_cycle_free is.

2. In the VM, classes that were created type finalized were not marked as
   cycle free because finalization did not consider them so cycle-freeness
   didn't matter.  We would therefore try to initialize these classes with
   DilReader::ReadPreliminaryClass and mutate types in them.  This causes
   them to have unfinalized types but still have is_type_finalized true.

3. Types in top-level procedures were not necessarily finalized.  A simple
   way to achieve this is to finalize each library's top-level class.

Review URL: https://chromereviews.googleplex.com/520437013 .
==========

to

==========
VM: [Kernel] Cherry-pick from dart-lang/kernel_sdk

Fix a trio of issues in type finalization.

1. We were using is_type_finalized to detect classes that had already been
   initialized by DilReader::ReadPreliminaryClass.  This state is not
   guaranteed by this function, however is_cycle_free is.

2. In the VM, classes that were created type finalized were not marked as
   cycle free because finalization did not consider them so cycle-freeness
   didn't matter.  We would therefore try to initialize these classes with
   DilReader::ReadPreliminaryClass and mutate types in them.  This causes
   them to have unfinalized types but still have is_type_finalized true.

3. Types in top-level procedures were not necessarily finalized.  A simple
   way to achieve this is to finalize each library's top-level class.

Review URL: https://chromereviews.googleplex.com/520437013 .

R=kmillikin@google.com

Committed:
https://github.com/dart-lang/sdk/commit/63af3f56ee9f8ff54c575a76cf6a3b124cac40bb
==========

[kustermann, 2016-11-21 15:38:38]

Committed patchset #3 (id:40001) manually as
63af3f56ee9f8ff54c575a76cf6a3b124cac40bb (presubmit successful).