On 2015/07/29 20:45:06, ofrobots wrote:
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.
Decided to make the suggested changes to remove recursion in a separate
change-list https://codereview.chromium.org/1265443003.
Once that is accepted, I will rebase this.
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.
