Thx, Hannes, here you go.
--Michael

https://codereview.chromium.org/40063002/diff/550001/src/allocation-site-scopes.cc
File src/allocation-site-scopes.cc (right):

https://codereview.chromium.org/40063002/diff/550001/src/allocation-site-scopes.cc#newcode100
src/allocation-site-scopes.cc:100:
ASSERT(FLAG_allocation_site_pretenuring);
On 2013/11/25 11:45:52, Hannes Payer wrote:
This assert has to hold in that case, please remove it.
I think it would make sense to assert that the type of the object is
JSObject or
JSArray.

Done.

https://codereview.chromium.org/40063002/diff/550001/src/allocation-site-scopes.cc#newcode102
src/allocation-site-scopes.cc:102: PrintF("*** Creating Memento for
JSObject/JSArray %p\n",
On 2013/11/25 11:45:52, Hannes Payer wrote:
Can we determine JSObject/JSArray dynamically?

Indeed. In fact, I ordered this code in a better way, we only need one
print statement and nesting can be reduced.

https://codereview.chromium.org/40063002/diff/550001/src/heap.cc
File src/heap.cc (right):

https://codereview.chromium.org/40063002/diff/550001/src/heap.cc#newcode514
src/heap.cc:514: if (cur->IsAllocationSite()) {
On 2013/11/25 11:45:52, Hannes Payer wrote:
I guess the if is not needed.

Done.

https://codereview.chromium.org/40063002/diff/550001/src/heap.cc#newcode4403
src/heap.cc:4403: reinterpret_cast<Address>(result) +
map->instance_size());
On 2013/11/25 11:45:52, Hannes Payer wrote:
Is there a way to increase the memento count at memento creation time?
Right now
it seems to be a bit fragile to manually increase the counter after
creating the
memento.

I consolidated the two heap.cc memento construction sites in
InitializeAllocationMemento, so there is some encapsulation. The
remaining site that creates a memento is in
HGraphBuilder::BuildJSArrayHeader(). Obviously the method is different
(hydrogen nodes).

BTW, you prompted me to go look at BuildJSArrayHeader() and I found that
I wasn't skipping an overflow check that I should, so I fixed that. I
definitely don't want a HeapNumber in the memento count field!

https://codereview.chromium.org/40063002/diff/550001/src/hydrogen.cc
File src/hydrogen.cc (right):

https://codereview.chromium.org/40063002/diff/550001/src/hydrogen.cc#newcode2249
src/hydrogen.cc:2249: BuildCreateAllocationMemento(array,
On 2013/11/25 11:45:52, Hannes Payer wrote:
I guess it would make sense to make the code block below part of
BuildCreateAllocationMemento. WDYT?

The reason I left it out of there was that constructed arrays don't need
this code until they are participating in the
alloc-site-based-pretenuring regime, and I don't want that code to pay
the performance penalty until it's getting the reward too. The
alternative is to add a boolean parameter to
BuildCreateAllocationMemento, like increment_create_count, but it would
only be necessary for a short time in the code base, until constructed
arrays are part of pretenuring.

https://codereview.chromium.org/40063002/diff/550001/src/hydrogen.cc#newcode2264
src/hydrogen.cc:2264: // no write barrier needed to store a smi.
On 2013/11/25 11:45:52, Hannes Payer wrote:
"No"

Done.

https://codereview.chromium.org/40063002/diff/550001/src/mark-compact.h
File src/mark-compact.h (right):

https://codereview.chromium.org/40063002/diff/550001/src/mark-compact.h#newcode742
src/mark-compact.h:742: // Special case for processing weak references
in a compacting collection.
On 2013/11/25 11:45:52, Hannes Payer wrote:
"compacting" -> "full"

Done.

https://codereview.chromium.org/40063002/

--
--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev
--- You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
For more options, visit https://groups.google.com/groups/opt_out.

Reply via email to