Makes new space iterable by filling up the mutator's TLAB

Makes new space iterable by filling up the mutator's TLAB

This reverts commit bb6203dadefc1bbe613b92ba25a91da06d55c194 and
adds changes FlushTLS.

Now, FlushTLS will fill the mutator thread's TLAB instead of
changing top_ in the Scavenger. This should prevent the assertion

(thread->end() == 0) || (thread->end() == top_)

in TryAllocateInTLAB in runtime/vm/scavenger.h due to a race on top_.

R=asiva@google.com, rmacnak@google.com, zra@google.com

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

[danunez, 2017-08-03 20:12:14]

So far, the assertions have not triggered in my testing.

I wanted to put this up for review now. Maybe someone will see something I
missed.

[rmacnak, 2017-08-03 20:31:41]

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

https://codereview.chromium.org/2991343003/diff/1/runtime/vm/heap.cc#newcode65
runtime/vm/heap.cc:65: if (end == new_space_.end()) {
Do we need this check? Won't this only happen if the remaining size is already
0?

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

https://codereview.chromium.org/2991343003/diff/1/runtime/vm/scavenger.cc#new...
runtime/vm/scavenger.cc:409: Thread* thread = isolate->mutator_thread();
We abandoned the TLAB at the beginning of the scavenge, so we should be about to
assert it has no TLAB here.

https://codereview.chromium.org/2991343003/diff/1/runtime/vm/scavenger.cc#new...
runtime/vm/scavenger.cc:936: if (heap_->isolate()->IsMutatorThreadScheduled()) {
Isn't this redundant with the abdanon-tlab in Scavenge()?

[danunez, 2017-08-03 20:39:21]

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

https://codereview.chromium.org/2991343003/diff/1/runtime/vm/heap.cc#newcode65
runtime/vm/heap.cc:65: if (end == new_space_.end()) {
On 2017/08/03 20:31:40, rmacnak wrote:
> Do we need this check? Won't this only happen if the remaining size is already
> 0?

As discussed, the TLAB may not be full. This check only checks if this TLAB
happens end right when the new space ends.

Also as discussed, This case will be removed in favor of treating each TLAB
equally. Therefore, any TLAB will be filled up through this function.

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

https://codereview.chromium.org/2991343003/diff/1/runtime/vm/scavenger.cc#new...
runtime/vm/scavenger.cc:409: Thread* thread = isolate->mutator_thread();
On 2017/08/03 20:31:40, rmacnak wrote:
> We abandoned the TLAB at the beginning of the scavenge, so we should be about
to
> assert it has no TLAB here.

Yes. That is true. I will add an assertion and remove lines 410 and 411.

https://codereview.chromium.org/2991343003/diff/1/runtime/vm/scavenger.cc#new...
runtime/vm/scavenger.cc:936: if (heap_->isolate()->IsMutatorThreadScheduled()) {
On 2017/08/03 20:31:40, rmacnak wrote:
> Isn't this redundant with the abdanon-tlab in Scavenge()?

The call to abandon-tlab is redundant and will be removed in favor of a single
call in Scavenge.

[rmacnak, 2017-08-04 17:31:59]

lgtm w/ comment

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

https://codereview.chromium.org/2991343003/diff/20001/runtime/vm/heap.cc#newc...
runtime/vm/heap.cc:61: ASSERT(Thread::Current()->no_safepoint_scope_depth() ==
0);
Fails from e.g., Heap::IterateObjects.

I would leave this out until multiple threads have TLABs.

[danunez, 2017-08-04 17:36:48]

Description was changed from

==========
Makes new space iterable by filling up the mutator's TLAB

This reverts commit bb6203dadefc1bbe613b92ba25a91da06d55c194 and
adds changes FlushTLS.

Now, FlushTLS will fill the mutator thread's TLAB instead of
changing top_ in the Scavenger. This should prevent the assertion

(thread->end() == 0) || (thread->end() == top_)

in TryAllocateInTLAB in runtime/vm/scavenger.h due to a race on top_.

R=rmacnak@google.com,asiva@google.com,zra@google.com
==========

to

==========
Makes new space iterable by filling up the mutator's TLAB

This reverts commit bb6203dadefc1bbe613b92ba25a91da06d55c194 and
adds changes FlushTLS.

Now, FlushTLS will fill the mutator thread's TLAB instead of
changing top_ in the Scavenger. This should prevent the assertion

(thread->end() == 0) || (thread->end() == top_)

in TryAllocateInTLAB in runtime/vm/scavenger.h due to a race on top_.

R=asiva@google.com, rmacnak@google.com, zra@google.com

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

[danunez, 2017-08-04 17:36:50]

Committed patchset #3 (id:40001) manually as
8b6fcf50e85d030dd7779e3bf40733a76fc95b0c (presubmit successful).