More cleanups for background compilation.

More cleanups for background compilation.

BUG=
R=hausner@google.com

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

[srdjan, 2015-09-04 23:04:17]

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

[hausner, 2015-09-05 01:22:10]

LGTM

I am getting concerned that one change after another, we add more garbage to old
space. I thought the idea was that only long-lived objects should be allocated
in old space.

Is there a metric we can use and observe (on golem for example) the shows us
whether this is becoming a real problem?

https://codereview.chromium.org/1325373004/diff/20001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/1325373004/diff/20001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:206: segment = String::SubString(name, start_pos, (i -
start_pos), Heap::kOld);
We are starting to litter the old heap space with lots of garbage that will not
survive. Doesn't this pervert the idea of separating old and new space? By the
time we are done with these changes, what will still be allocated in new space?

https://codereview.chromium.org/1325373004/diff/20001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:9093: return Symbols::FromConcatAll(pieces);
Can't you use the Symbols::FromConcat() here? There are only two pieces.

[srdjan, 2015-09-06 05:23:53]

On 2015/09/05 01:22:10, hausner wrote:
> LGTM
> 
> I am getting concerned that one change after another, we add more garbage to
old
> space. I thought the idea was that only long-lived objects should be allocated
> in old space.
> 
> Is there a metric we can use and observe (on golem for example) the shows us
> whether this is becoming a real problem?

Yes, only long lived objects should be allocated in the old space. The exception
are temporary type objects. The plan is to use snapshots only for production
mode, in which case  all the type analysis will be skipped and the temporary
type objects will not be allocated in production mode  (or at least their number
will be significantly reduced).

Any other allocation of short-lived objects in the old space is a mistake.

I assume that excessive allocation in old-space should show up as performance
degradation, as tracked.

> 
> https://codereview.chromium.org/1325373004/diff/20001/runtime/vm/object.cc
> File runtime/vm/object.cc (right):
> 
>
https://codereview.chromium.org/1325373004/diff/20001/runtime/vm/object.cc#ne...
> runtime/vm/object.cc:206: segment = String::SubString(name, start_pos, (i -
> start_pos), Heap::kOld);
> We are starting to litter the old heap space with lots of garbage that will
not
> survive. Doesn't this pervert the idea of separating old and new space? By the
> time we are done with these changes, what will still be allocated in new
space?
> 
>
https://codereview.chromium.org/1325373004/diff/20001/runtime/vm/object.cc#ne...
> runtime/vm/object.cc:9093: return Symbols::FromConcatAll(pieces);
> Can't you use the Symbols::FromConcat() here? There are only two pieces.

[srdjan, 2015-09-06 05:24:10]

https://codereview.chromium.org/1325373004/diff/20001/runtime/vm/object.cc
File runtime/vm/object.cc (right):

https://codereview.chromium.org/1325373004/diff/20001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:206: segment = String::SubString(name, start_pos, (i -
start_pos), Heap::kOld);
On 2015/09/05 01:22:10, hausner wrote:
> We are starting to litter the old heap space with lots of garbage that will
not
> survive. Doesn't this pervert the idea of separating old and new space? By the
> time we are done with these changes, what will still be allocated in new
space?

I assumed this function was used only by the disassembler, but it seems not to
be the case. I will revert the change for this function.

https://codereview.chromium.org/1325373004/diff/20001/runtime/vm/object.cc#ne...
runtime/vm/object.cc:9093: return Symbols::FromConcatAll(pieces);
On 2015/09/05 01:22:10, hausner wrote:
> Can't you use the Symbols::FromConcat() here? There are only two pieces.

Yes

[srdjan, 2015-09-06 18:11:05]

Committed patchset #4 (id:60001) manually as
cba48a6ab6ea08a72803716153c433059bebaf0e (presubmit successful).