- Start using the collected store buffer entries to find

- Start using the collected store buffer entries to find
  old->new references during Scavenge.
Committed: https://code.google.com/p/dart/source/detail?r=9791

[siva, 2012-07-19 17:36:06]

some minor questions but otherwise it looks good.

https://chromiumcodereview.appspot.com/10797021/diff/1/vm/scavenger.cc
File vm/scavenger.cc (right):

https://chromiumcodereview.appspot.com/10797021/diff/1/vm/scavenger.cc#newcod...
vm/scavenger.cc:271: heap_->IterateOldPointers(&verify_store_buffer_visitor);
This verification doesn't quite ensure that the store buffers have the correct
data right.

Would it make sense to have a verification step in Scavenger::Prologue which
will iterate old pointers and if it has a new object ensure that it exists in
the store buffer.

https://chromiumcodereview.appspot.com/10797021/diff/1/vm/scavenger.cc#newcod...
vm/scavenger.cc:284: // Iterating though the store buffers.
spelling!!

https://chromiumcodereview.appspot.com/10797021/diff/1/vm/scavenger.cc#newcod...
vm/scavenger.cc:332: }
Instead of processing this StoreBufferBlock separately here would it make sense
to just call ProcessBuffer at the start of this function (that way all the
duplicates in this block would get deduped) and accounted for in the earlier
loop.

https://chromiumcodereview.appspot.com/10797021/diff/1/vm/store_buffer.cc
File vm/store_buffer.cc (right):

https://chromiumcodereview.appspot.com/10797021/diff/1/vm/store_buffer.cc#new...
vm/store_buffer.cc:64: }
If we overflow again before the interrupt actually happens we will end up
scheduling another interrupt causing back to back GCs. Is this scenario
possible?

[Ivan Posva, 2012-07-20 15:37:46]

https://chromiumcodereview.appspot.com/10797021/diff/1/vm/scavenger.cc
File vm/scavenger.cc (right):

https://chromiumcodereview.appspot.com/10797021/diff/1/vm/scavenger.cc#newcod...
vm/scavenger.cc:271: heap_->IterateOldPointers(&verify_store_buffer_visitor);
On 2012/07/19 17:36:06, asiva wrote:
> This verification doesn't quite ensure that the store buffers have the correct
> data right.
> 
> Would it make sense to have a verification step in Scavenger::Prologue which
> will iterate old pointers and if it has a new object ensure that it exists in
> the store buffer.

Discussed offline: Suggestion would work and has the same effect as the
currently implemented algorithm. The advantage of doing it after the Scavenge is
not having to search in the store buffers.

https://chromiumcodereview.appspot.com/10797021/diff/1/vm/scavenger.cc#newcod...
vm/scavenger.cc:284: // Iterating though the store buffers.
On 2012/07/19 17:36:06, asiva wrote:
> spelling!!

Done.

https://chromiumcodereview.appspot.com/10797021/diff/1/vm/scavenger.cc#newcod...
vm/scavenger.cc:332: }
On 2012/07/19 17:36:06, asiva wrote:
> Instead of processing this StoreBufferBlock separately here would it make
sense
> to just call ProcessBuffer at the start of this function (that way all the
> duplicates in this block would get deduped) and accounted for in the earlier
> loop.

The number of addresses visited here is limited so I would rather keep it as it
is. Also I want to avoid adding more addresses to the dedup sets as growth in
them would trigger the interrupt again.

https://chromiumcodereview.appspot.com/10797021/diff/1/vm/store_buffer.cc
File vm/store_buffer.cc (right):

https://chromiumcodereview.appspot.com/10797021/diff/1/vm/store_buffer.cc#new...
vm/store_buffer.cc:64: }
On 2012/07/19 17:36:06, asiva wrote:
> If we overflow again before the interrupt actually happens we will end up
> scheduling another interrupt causing back to back GCs. Is this scenario
> possible?

You can schedule an interrupt as many times as you want. The handler will be
only called once when the interrupt is pending. Once the interrupt is cleared
and in process of being handled you can schedule new interrupts. As far as I can
see a back-to-back Scavenge can happen only if the scavenging itself caused an
overflow. I do have a plan for avoiding this, but it is not part of this CL.

[Ivan Posva, 2012-07-20 21:33:48]

PTAL