On 2015/07/29 21:56:00, ofrobots wrote:
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.
Superceded by https://codereview.chromium.org/1274453002/.
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.
