On 2015/07/24 06:32:10, Hannes Payer wrote:
https://codereview.chromium.org/1252053003/diff/1/src/heap/spaces.cc
File src/heap/spaces.cc (right):
https://codereview.chromium.org/1252053003/diff/1/src/heap/spaces.cc#newcode1478
src/heap/spaces.cc:1478: int aligned_size_in_bytes = size_in_bytes +
alignment_size;
On 2015/07/23 17:41:06, ofrobots wrote:
> On 2015/07/23 16:31:34, ofrobots wrote:
> > On 2015/07/23 10:58:54, Hannes Payer wrote:
> > > I think we do not need the complicated machinery to update new
inline
> > allocation
> > > limit. Why don't we just allocate, get the new top pointer after
that,
> perform
> > > the Step, and call UpdateInlineAllocationLimit(0)?
> >
> > Okay, let me look into this.
>
> The reason this complicated machinery exists is to avoid an infinite
loop.
> Calling allocate first before doing UpdateInlineAllocationLimit makes a
> recursive infinite loop between AllocateRaw* and SlowAllocateRaw. To
call
> UpdateInlineAllocationLimit we need to do this complicated machinery to
> guesstimate the aligned_size_in_bytes that the allocation would
allocate.
>
> This code is a bit too wacky. The challenge is to find incremental
steps to
make
> it better.
>
> I stand by my earlier assertion that it does not make a lot of logical
sense
to
> do a step in the `if (AddFreshPage())` clause because there is no clean
way
to
> avoid double counting. One option would be for me to add an argument to
> AllocateRaw* to distinguish the recursive call from here in
SlowAllocateRate,
> but I really dislike that option.
Arg, yes. That is the reason why it is complicated. OK, let's keep it.
This
also
complicates the AddFreshPage case.
Adding a parameter to AllocateRaw to indicate the recursion is not nice.
One way to clean that up would be to get rid of this recursion.
SlowAllocateRaw
(renamed to EnsureAllocation) could basically make sure that there is
room for
the allocation if possible and could just return to AllocateRaw* which
performs
the allocation. I think then it should be possible to account for the
right
allocated bytes. Similar cases as before 1) just increase the limit if
allowed
2) a new page is needed: don't account for the leftover memory at the end
of
the
page and the page header. As a result, each call to allocation would just
perform one step and would update the top_on_previous_step counter once.
WDYT?
SGTM. Will update the patches.
https://codereview.chromium.org/1252053003/
--
--
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/d/optout.
