When using SDK outline, transplant its names into the name root.

When using SDK outline, transplant its names into the name root.

It's a bit messy that we have to break immutability of CanonicalName(s)
here. The alternative is not re-load SDK outline from bytes every time.
It is relatively expensive for small tests, although probably relatively
less expensive for large apps when we load hundreds of other libraries.

Thoughts?

R=ahe@google.com, paulberry@google.com, sigmund@google.com
BUG= https://github.com/dart-lang/sdk/issues/30409

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

[scheglov, 2017-08-11 17:21:30]

Description was changed from

==========
When using SDK outline, transplant its names into the name root.

It's a bit messy that we have to break immutability of CanonicalName(s)
here. The alternative is not re-load SDK outline from bytes every time.
It is relatively expensive for small tests, although probably relatively
less expensive for large apps when we load hundreds of other libraries.

Thoughts?

R=ahe@google.com, paulberry@google.com, sigmund@google.com
BUG=
==========

to

==========
When using SDK outline, transplant its names into the name root.

It's a bit messy that we have to break immutability of CanonicalName(s)
here. The alternative is not re-load SDK outline from bytes every time.
It is relatively expensive for small tests, although probably relatively
less expensive for large apps when we load hundreds of other libraries.

Thoughts?

R=ahe@google.com, paulberry@google.com, sigmund@google.com
BUG= https://github.com/dart-lang/sdk/issues/30409
==========

[Siggi Cherem (dart-lang), 2017-08-11 17:27:27]

https://codereview.chromium.org/2995773002/diff/1/pkg/front_end/lib/src/incre...
File pkg/front_end/lib/src/incremental/kernel_driver.dart (right):

https://codereview.chromium.org/2995773002/diff/1/pkg/front_end/lib/src/incre...
pkg/front_end/lib/src/incremental/kernel_driver.dart:367: _sdkOutline = new
Program();
I'm not sure I follow - wouldn't it work instead if we pass `nameRoot` here to
this constructor instead?

[scheglov, 2017-08-11 17:29:23]

https://codereview.chromium.org/2995773002/diff/1/pkg/front_end/lib/src/incre...
File pkg/front_end/lib/src/incremental/kernel_driver.dart (right):

https://codereview.chromium.org/2995773002/diff/1/pkg/front_end/lib/src/incre...
pkg/front_end/lib/src/incremental/kernel_driver.dart:367: _sdkOutline = new
Program();
On 2017/08/11 17:27:27, Siggi Cherem (dart-lang) wrote:
> I'm not sure I follow - wouldn't it work instead if we pass `nameRoot` here to
> this constructor instead?

We use different "nameRoot" each time.

It will work, if we re-load SDK outline each time form binary.

[Siggi Cherem (dart-lang), 2017-08-11 17:31:16]

On 2017/08/11 17:29:23, scheglov wrote:
>
https://codereview.chromium.org/2995773002/diff/1/pkg/front_end/lib/src/incre...
> File pkg/front_end/lib/src/incremental/kernel_driver.dart (right):
> 
>
https://codereview.chromium.org/2995773002/diff/1/pkg/front_end/lib/src/incre...
> pkg/front_end/lib/src/incremental/kernel_driver.dart:367: _sdkOutline = new
> Program();
> On 2017/08/11 17:27:27, Siggi Cherem (dart-lang) wrote:
> > I'm not sure I follow - wouldn't it work instead if we pass `nameRoot` here
to
> > this constructor instead?
> 
> We use different "nameRoot" each time.
> 
> It will work, if we re-load SDK outline each time form binary.

I see - I think I prefer doing so. We are doing the same thing for every cached
.dill we have in the byte store (via the dilltarget), right?

[Siggi Cherem (dart-lang), 2017-08-11 17:32:36]

On 2017/08/11 17:31:16, Siggi Cherem (dart-lang) wrote:
> On 2017/08/11 17:29:23, scheglov wrote:
> >
>
https://codereview.chromium.org/2995773002/diff/1/pkg/front_end/lib/src/incre...
> > File pkg/front_end/lib/src/incremental/kernel_driver.dart (right):
> > 
> >
>
https://codereview.chromium.org/2995773002/diff/1/pkg/front_end/lib/src/incre...
> > pkg/front_end/lib/src/incremental/kernel_driver.dart:367: _sdkOutline = new
> > Program();
> > On 2017/08/11 17:27:27, Siggi Cherem (dart-lang) wrote:
> > > I'm not sure I follow - wouldn't it work instead if we pass `nameRoot`
here
> to
> > > this constructor instead?
> > 
> > We use different "nameRoot" each time.
> > 
> > It will work, if we re-load SDK outline each time form binary.
> 
> I see - I think I prefer doing so. We are doing the same thing for every
cached
> .dill we have in the byte store (via the dilltarget), right?

(I mean, it would be as expensive as it was when we built the sdk from sources
the first time, then any subsequent time we would deserialize it each time from
binary as well)

[scheglov, 2017-08-11 17:32:45]

https://codereview.chromium.org/2995773002/diff/1/pkg/front_end/lib/src/incre...
File pkg/front_end/lib/src/incremental/kernel_driver.dart (right):

https://codereview.chromium.org/2995773002/diff/1/pkg/front_end/lib/src/incre...
pkg/front_end/lib/src/incremental/kernel_driver.dart:367: _sdkOutline = new
Program();
On 2017/08/11 17:29:23, scheglov wrote:
> On 2017/08/11 17:27:27, Siggi Cherem (dart-lang) wrote:
> > I'm not sure I follow - wouldn't it work instead if we pass `nameRoot` here
to
> > this constructor instead?
> 
> We use different "nameRoot" each time.
> 
> It will work, if we re-load SDK outline each time form binary.

Yes, that's what I was saying - we do this for every other library.
It's just SDK that it special - it does not depend on any other library, so
potentially we could keep it loaded. But it's messy. I will rework.

[scheglov, 2017-08-11 18:50:21]

PTAL

[Siggi Cherem (dart-lang), 2017-08-11 18:55:29]

lgtm

[scheglov, 2017-08-11 19:00:43]

Description was changed from

==========
When using SDK outline, transplant its names into the name root.

It's a bit messy that we have to break immutability of CanonicalName(s)
here. The alternative is not re-load SDK outline from bytes every time.
It is relatively expensive for small tests, although probably relatively
less expensive for large apps when we load hundreds of other libraries.

Thoughts?

R=ahe@google.com, paulberry@google.com, sigmund@google.com
BUG= https://github.com/dart-lang/sdk/issues/30409
==========

to

==========
When using SDK outline, transplant its names into the name root.

It's a bit messy that we have to break immutability of CanonicalName(s)
here. The alternative is not re-load SDK outline from bytes every time.
It is relatively expensive for small tests, although probably relatively
less expensive for large apps when we load hundreds of other libraries.

Thoughts?

R=ahe@google.com, paulberry@google.com, sigmund@google.com
BUG= https://github.com/dart-lang/sdk/issues/30409

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

[scheglov, 2017-08-11 19:00:44]

Committed patchset #2 (id:20001) manually as
ada4db3fb6c7cb067e0f0bb98f9ec23d9362e5a5 (presubmit successful).