Use clustered serialization for full snapshots.

Use clustered serialization for full snapshots.

Retain the recursive-descent format for script and message snapshots.

Flutter gallery app on a Nexus 4
 - Snapshot, excluding instruction/rodata sections 3429177 -> 2874997B (-16%)
 - InitOnce 54.1 -> 8.4ms (-84%)
 - InitializeIsolate 374.3 -> 170.8ms (-54%)

R=asiva@google.com

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

[rmacnak, 2016-06-11 00:45:20]

Description was changed from

==========
stash

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

to

==========
Use clustered serialization for full snapshots.
==========

[rmacnak, 2016-06-14 00:12:35]

Description was changed from

==========
Use clustered serialization for full snapshots.
==========

to

==========
Use clustered serialization for full snapshots.

Retain the recursive-descent format for script and message snapshots.
==========

[rmacnak, 2016-06-14 17:03:40]

Description was changed from

==========
Use clustered serialization for full snapshots.

Retain the recursive-descent format for script and message snapshots.
==========

to

==========
Use clustered serialization for full snapshots.

Retain the recursive-descent format for script and message snapshots.
==========

[rmacnak, 2016-06-14 17:59:57]

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

[rmacnak, 2016-06-14 18:29:33]

Description was changed from

==========
Use clustered serialization for full snapshots.

Retain the recursive-descent format for script and message snapshots.
==========

to

==========
Use clustered serialization for full snapshots.

Retain the recursive-descent format for script and message snapshots.

Flutter gallery app on a Nexus 4
 - Snapshot, excluding instruction/rodata sections 3429177 -> 2874997B (-16%)
 - InitOnce 54.1 -> 8.4ms (-84%)
 - InitializeIsolate 374.3 -> 170.8ms (-54%)
==========

[Florian Schneider, 2016-06-14 20:32:18]

fschneider@google.com changed reviewers:
+ fschneider@google.com

[Florian Schneider, 2016-06-14 20:32:20]

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/raw_object.h
File runtime/vm/raw_object.h (right):

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/raw_object....
runtime/vm/raw_object.h:754: friend class UnresolvedClassSerializationCluster;
Have you tried making these friend declarations part of the
RAW_HEAP_OBJECT_IMPLEMENTATION macro?

[rmacnak, 2016-06-21 17:53:03]

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/raw_object.h
File runtime/vm/raw_object.h (right):

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/raw_object....
runtime/vm/raw_object.h:754: friend class UnresolvedClassSerializationCluster;
On 2016/06/14 20:32:20, Florian Schneider wrote:
> Have you tried making these friend declarations part of the
> RAW_HEAP_OBJECT_IMPLEMENTATION macro?

Done.

[siva, 2016-06-21 23:51:43]

Some initial comments (Still continuing review).

https://codereview.chromium.org/2032153003/diff/260001/runtime/lib/stacktrace.cc
File runtime/lib/stacktrace.cc (right):

https://codereview.chromium.org/2032153003/diff/260001/runtime/lib/stacktrace...
runtime/lib/stacktrace.cc:71: }
Is this another utility method for use from gdb, if so please document it and
why it is different from _printCurrentStackTrace, also the name
_printCurrentStackTrace2 is weird.

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/class_table.cc
File runtime/vm/class_table.cc (right):

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/class_table...
runtime/vm/class_table.cc:211: ASSERT(table_[index] == 0);
This piece of code is duplicated here and above, maybe you should have a method
ClassTable::EnsureCapacity(intptr_t index); and move this code  there.

The callers can then use this pattern:
  EnsureCapcity(index);
  RegisterAt(index, rawcls);

Then you may not need two versions of RegisterAt

I am always nervous about having a raw pointer being available across a big
chunk of code like this.

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/clustered_s...
File runtime/vm/clustered_snapshot.h (right):

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:8: #include "platform/assert.h"
blank line after this?

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:20: #include "vm/version.h"
doesn't seem to be in alphabetic order

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:23: 
 Add comment,

// Forward declarations.

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:31: // class instead for each object, and to
allow filling the objects in a tight
instead of once per object,

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:32: // loop. The snapshot has two major
sections: the first describes how to
two sections ? (there are no minor sections right).

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:38: // initialization/fill secton is read for
cluster, using the indices into the
for each cluster

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:76: virtual void PostLoad(const Array& refs,
Snapshot::Kind kind, Zone* zone) { }
Why do you have the kind parameter here if this is done always for full
snapshots?

[rmacnak, 2016-06-22 19:42:18]

https://codereview.chromium.org/2032153003/diff/260001/runtime/lib/stacktrace.cc
File runtime/lib/stacktrace.cc (right):

https://codereview.chromium.org/2032153003/diff/260001/runtime/lib/stacktrace...
runtime/lib/stacktrace.cc:71: }
On 2016/06/21 23:51:42, siva wrote:
> Is this another utility method for use from gdb, if so please document it and
> why it is different from _printCurrentStackTrace, also the name
> _printCurrentStackTrace2 is weird.

Renamed to _printCurrentStacktraceNoSafepoint.

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/class_table.cc
File runtime/vm/class_table.cc (right):

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/class_table...
runtime/vm/class_table.cc:211: ASSERT(table_[index] == 0);
On 2016/06/21 23:51:43, siva wrote:
> This piece of code is duplicated here and above, maybe you should have a
method
> ClassTable::EnsureCapacity(intptr_t index); and move this code  there.
> 
> The callers can then use this pattern:
>   EnsureCapcity(index);
>   RegisterAt(index, rawcls);
> 
> Then you may not need two versions of RegisterAt
> 
> I am always nervous about having a raw pointer being available across a big
> chunk of code like this.

Factored out as AllocateIndex, which ensures capacity and bumps top_.

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/clustered_s...
File runtime/vm/clustered_snapshot.h (right):

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:8: #include "platform/assert.h"
On 2016/06/21 23:51:43, siva wrote:
> blank line after this?

We generally only separate a cc's corresponding header file and system header
files.

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:20: #include "vm/version.h"
On 2016/06/21 23:51:43, siva wrote:
> doesn't seem to be in alphabetic order

Re-sorted.

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:23: 
On 2016/06/21 23:51:43, siva wrote:
>  Add comment,
> 
> // Forward declarations.

Done.

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:31: // class instead for each object, and to
allow filling the objects in a tight
On 2016/06/21 23:51:43, siva wrote:
> instead of once per object,

Done.

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:32: // loop. The snapshot has two major
sections: the first describes how to
On 2016/06/21 23:51:43, siva wrote:
> two sections ? (there are no minor sections right).

Well, in some sense the major sections for allocate and initialize are
subdivided into minor sections for each cluster.

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:38: // initialization/fill secton is read for
cluster, using the indices into the
On 2016/06/21 23:51:43, siva wrote:
> for each cluster

Done.

https://codereview.chromium.org/2032153003/diff/260001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:76: virtual void PostLoad(const Array& refs,
Snapshot::Kind kind, Zone* zone) { }
On 2016/06/21 23:51:43, siva wrote:
> Why do you have the kind parameter here if this is done always for full
> snapshots?

Some actions are taken only for AOT snapshots.

[siva, 2016-06-30 00:06:08]

lgtm with a few comments.

https://codereview.chromium.org/2032153003/diff/280001/runtime/vm/clustered_s...
File runtime/vm/clustered_snapshot.h (right):

https://codereview.chromium.org/2032153003/diff/280001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:181:
Write<intptr_t>(reinterpret_cast<intptr_t>(object));
How does this work when the snapshot is generated on a 64 bit system and then
read into a 32 bit system?

https://codereview.chromium.org/2032153003/diff/280001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:300: return reinterpret_cast<RawSmi*>(index);
See question about reading a snapshot generated on a 64 bit system and read on a
32 bit system.

https://codereview.chromium.org/2032153003/diff/300001/runtime/lib/stacktrace.cc
File runtime/lib/stacktrace.cc (right):

https://codereview.chromium.org/2032153003/diff/300001/runtime/lib/stacktrace...
runtime/lib/stacktrace.cc:72: }
Do we need the other version, why not make your version
_printCurrentStacktrace() and get rid of the existing version.

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
File runtime/vm/clustered_snapshot.cc (right):

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.cc:54: ClassSerializationCluster() { }
Maybe 
ClassSerializationCluster() ::
  predefined_(some_initial_capacity),
  objects_(some_other_initial_capacity) { }

where the capacity is based on what the array grows to currently when you
generate a snapshot.

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.cc:111:
s->Write<uint16_t>(cls->ptr()->state_bits_);                               \
Why not make this a static helper function and call it below, otherwise this
will become a very big function.

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.cc:178: }
Predefined classes are allocated in Object::Init using the regular Class::New
function so they should be already null initialized right why do we need to do
this null loop again?

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.cc:189: cls->ptr()->state_bits_ =
d->Read<uint16_t>();
This piece of code is repeated why not make a helper function.

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.cc:442: }
The WriteAlloc function seems similar in each of the SerializationClusters can
it be made a function in the base class SerilizationCluster with a parameters
cid and length

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.cc:474: }
The ReadAlloc function seems similar in each of the DeserializationClusters can
it be made a function in the base class DeserializationCluster with a parameter
(size).

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.cc:915: };
two blank lines between all these classes in this file?

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.cc:1553: // s->NoteUntracedRef();
Code commented out, can it be deleted?

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.cc:4045: clusters_by_cid_[i] = NULL;
why not just call NewClusterForClass(i) here instead of initializing to NULL
wouldn't we encounter all the class ids in a full snapshot anyway?

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.cc:4635: HeapLocker hl(thread(),
heap_->old_space());
Why do you need to lock the heap pages, no other threads should be running while
we read the VM snapshot and isolate snapshot right?

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.cc:4815: // Reuse the current vm isolate.
Why have this empty block here?

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
File runtime/vm/clustered_snapshot.h (right):

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:107: ASSERT(next_ref_index_ != 0);
ASSERT(heap_->GetObjectId(object) == 0);

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:115: ASSERT(heap_->GetObjectId(object) == 1);
how do you end up with an initial id of 1?

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:118: next_ref_index_++;
AssignRef and AssignRefNotTraced seem to be identical except for one ASSERT, why
not just have one?

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:200: Write<intptr_t>((id << 1) | 1);
We should probably use an enum instead of 1, something like:

enum SerializedHeaderType {
  kInlined  = 0x1,
  kObjectId = 0x3,
};
and SerializedHeaderTag & SerializedHeaderData

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:207: void WriteCid(intptr_t cid) {
COMPILE_ASSERT(RawObject::kClassIdTagSize == 32);

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:310: intptr_t ReadCid() {
Ditto COMPILE_ASSERT....

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:357: uint8_t** vm_isolate_snapshot_buffer() {
const {

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:361: uint8_t** isolate_snapshot_buffer() {
const

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:444: Thread* thread) :
formatting seems odd.

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/dart.cc
File runtime/vm/dart.cc (right):

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/dart.cc#new...
runtime/vm/dart.cc:36: #include "vm/clustered_snapshot.h"
Should clustered_snapshot.h be included inside snapshot.h ?

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/snapshot_te...
File runtime/vm/snapshot_test.cc (right):

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/snapshot_te...
runtime/vm/snapshot_test.cc:18: #include "vm/clustered_snapshot.h"
ditto question about including this inside 'snapshot.h'

[rmacnak, 2016-06-30 01:39:22]

https://codereview.chromium.org/2032153003/diff/280001/runtime/vm/clustered_s...
File runtime/vm/clustered_snapshot.h (right):

https://codereview.chromium.org/2032153003/diff/280001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:181:
Write<intptr_t>(reinterpret_cast<intptr_t>(object));
On 2016/06/30 00:06:07, siva wrote:
> How does this work when the snapshot is generated on a 64 bit system and then
> read into a 32 bit system?

It doesn't. Added word size to the features descriptor checked on snapshot load.

https://codereview.chromium.org/2032153003/diff/300001/runtime/lib/stacktrace.cc
File runtime/lib/stacktrace.cc (right):

https://codereview.chromium.org/2032153003/diff/300001/runtime/lib/stacktrace...
runtime/lib/stacktrace.cc:72: }
On 2016/06/30 00:06:07, siva wrote:
> Do we need the other version, why not make your version
> _printCurrentStacktrace() and get rid of the existing version.

The existing version is often preferable when it works because you get line
numbers. The new version is helpful for lower-level bugs.

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
File runtime/vm/clustered_snapshot.cc (right):

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.cc:54: ClassSerializationCluster() { }
On 2016/06/30 00:06:08, siva wrote:
> Maybe 
> ClassSerializationCluster() ::
>   predefined_(some_initial_capacity),
>   objects_(some_other_initial_capacity) { }
> 
> where the capacity is based on what the array grows to currently when you
> generate a snapshot.

For a full snapshot we actually should expect the size of the class table.

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.cc:111:
s->Write<uint16_t>(cls->ptr()->state_bits_);                               \
On 2016/06/30 00:06:08, siva wrote:
> Why not make this a static helper function and call it below, otherwise this
> will become a very big function.

Done.

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.cc:178: }
On 2016/06/30 00:06:07, siva wrote:
> Predefined classes are allocated in Object::Init using the regular Class::New
> function so they should be already null initialized right why do we need to do
> this null loop again?

True, removed.

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.cc:915: };
On 2016/06/30 00:06:08, siva wrote:
> two blank lines between all these classes in this file?

Done.

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.cc:1553: // s->NoteUntracedRef();
On 2016/06/30 00:06:08, siva wrote:
> Code commented out, can it be deleted?

-> TODO

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.cc:4045: clusters_by_cid_[i] = NULL;
On 2016/06/30 00:06:07, siva wrote:
> why not just call NewClusterForClass(i) here instead of initializing to NULL
> wouldn't we encounter all the class ids in a full snapshot anyway?

We only create a cluster if there are instances of the class included in the
snapshot. For user classes, they would only need a cluster if they have const
instances.

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.cc:4635: HeapLocker hl(thread(),
heap_->old_space());
On 2016/06/30 00:06:07, siva wrote:
> Why do you need to lock the heap pages, no other threads should be running
while
> we read the VM snapshot and isolate snapshot right?

Comes from the old full snapshot reading code. Removing this triggers asserts in
old_space->TryAllocateDataBumpLocked.

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.cc:4815: // Reuse the current vm isolate.
On 2016/06/30 00:06:08, siva wrote:
> Why have this empty block here?

Scoping the comment.

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
File runtime/vm/clustered_snapshot.h (right):

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:115: ASSERT(heap_->GetObjectId(object) == 1);
On 2016/06/30 00:06:08, siva wrote:
> how do you end up with an initial id of 1?

The 1 is a mark set by Push()

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:118: next_ref_index_++;
On 2016/06/30 00:06:08, siva wrote:
> AssignRef and AssignRefNotTraced seem to be identical except for one ASSERT,
why
> not just have one?

Merged.

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:200: Write<intptr_t>((id << 1) | 1);
On 2016/06/30 00:06:08, siva wrote:
> We should probably use an enum instead of 1, something like:
> 
> enum SerializedHeaderType {
>   kInlined  = 0x1,
>   kObjectId = 0x3,
> };
> and SerializedHeaderTag & SerializedHeaderData

Done.

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:207: void WriteCid(intptr_t cid) {
On 2016/06/30 00:06:08, siva wrote:
> COMPILE_ASSERT(RawObject::kClassIdTagSize == 32);

<= 32

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:310: intptr_t ReadCid() {
On 2016/06/30 00:06:08, siva wrote:
> Ditto COMPILE_ASSERT....

<= 32

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:357: uint8_t** vm_isolate_snapshot_buffer() {
On 2016/06/30 00:06:08, siva wrote:
> const {

Done.

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:361: uint8_t** isolate_snapshot_buffer() {
On 2016/06/30 00:06:08, siva wrote:
> const

Done.

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/clustered_s...
runtime/vm/clustered_snapshot.h:444: Thread* thread) :
On 2016/06/30 00:06:08, siva wrote:
> formatting seems odd.

Fixed. (was due to renaming)

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/dart.cc
File runtime/vm/dart.cc (right):

https://codereview.chromium.org/2032153003/diff/300001/runtime/vm/dart.cc#new...
runtime/vm/dart.cc:36: #include "vm/clustered_snapshot.h"
On 2016/06/30 00:06:08, siva wrote:
> Should clustered_snapshot.h be included inside snapshot.h ?

There's no use it from the script/message snapshot code.

[rmacnak, 2016-06-30 02:50:11]

Description was changed from

==========
Use clustered serialization for full snapshots.

Retain the recursive-descent format for script and message snapshots.

Flutter gallery app on a Nexus 4
 - Snapshot, excluding instruction/rodata sections 3429177 -> 2874997B (-16%)
 - InitOnce 54.1 -> 8.4ms (-84%)
 - InitializeIsolate 374.3 -> 170.8ms (-54%)
==========

to

==========
Use clustered serialization for full snapshots.

Retain the recursive-descent format for script and message snapshots.

Flutter gallery app on a Nexus 4
 - Snapshot, excluding instruction/rodata sections 3429177 -> 2874997B (-16%)
 - InitOnce 54.1 -> 8.4ms (-84%)
 - InitializeIsolate 374.3 -> 170.8ms (-54%)

R=asiva@google.com

Committed:
https://github.com/dart-lang/sdk/commit/524fbc1e0f9d61ff5da69bbed2b35745528de945
==========

[rmacnak, 2016-06-30 02:50:13]

Committed patchset #17 (id:320001) manually as
524fbc1e0f9d61ff5da69bbed2b35745528de945 (presubmit successful).