LGTM, just comment nits
https://codereview.chromium.org/170703002/diff/1/src/heap.h
File src/heap.h (right):
https://codereview.chromium.org/170703002/diff/1/src/heap.h#newcode688
src/heap.h:688: // pretenure state.
On 2014/02/26 09:47:41, mvstanton wrote:
On 2014/02/19 21:29:07, Hannes Payer wrote:
> What do you mean by "does not influence the pretenure state"?
I mean that the AllocationSite is not mined for information used to
determine
the Space for the allocation.
Perhaps there should be some renaming of these methods. The way they
function is
that a site is passed only so that a memento can be allocated with the
object.
Maybe the argument should be called "site_for_memento."
Just drop the "allocation_site does not influence the pretenure state."
https://codereview.chromium.org/170703002/diff/1/src/heap.h#newcode693
src/heap.h:693: AllocationSite* allocation_site = NULL);
On 2014/02/26 09:47:41, mvstanton wrote:
On 2014/02/19 21:29:07, Hannes Payer wrote:
> What about implementing an AllocationMode, similar to
HAllocationMode?
I am thinking small with this CL, focused on eliminating duplicate
code rather
than introducing new constructs.
OK, I guess we should implement an AllocationMode in a follow-up CL.
https://codereview.chromium.org/170703002/diff/1/src/heap.h#newcode774
src/heap.h:774: // points to the site. It doesn't influence pretenure
state.
On 2014/02/26 09:47:41, mvstanton wrote:
On 2014/02/19 21:29:07, Hannes Payer wrote:
> same comments as above
Indeed. And probably the fact that I have to explain this
(unsuccessfully at
that) implies that the API is broken anyway!
Just drop the "It doesn't influence pretenure state."
https://codereview.chromium.org/170703002/
--
--
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.