https://codereview.chromium.org/1252053003/diff/1/src/heap/spaces.cc
File src/heap/spaces.cc (left):
https://codereview.chromium.org/1252053003/diff/1/src/heap/spaces.cc#oldcode1489
src/heap/spaces.cc:1489: int bytes_allocated = static_cast<int>(old_top
- top_on_previous_step_);
On 2015/07/23 10:58:54, Hannes Payer wrote:
This code accounted for bytes_allocated before we switch over to a new
page.
When you remove that, you are going to account for potentially more
memory on
the next Step (because of fragmentation). I do not think we should
remove this
code.
Okay, I understand why this case is here in the first place: to avoid
counting the headers on the new page. Logically it was not making sense
why we were doing a step here because to reach this code we clearly
didn't reach the inline-allocation-limit.
Regardless, independent of the rest of my changes, there was a bug in
here. Consider the scenario where we reach the inline allocation limit
and there is no room left on the current page. In that scenario, we do a
IncremetalMark step, and then allocate using AllocateRaw[Un]Aligned
which recursively calls back to SlowAllocateRaw. This time we will reach
the AddFreshPage scenario and try to do a step again. Depending on
whether top_on_previous_step_ was updated before (original code) or
after (my change) AllocateRaw[Un]Aligned, we either 1) compute a
negative bytes allocated or 2) count the same bytes twice.
Logically, this is not a step, but rather we want to discount some part
of the linear allocation that we would otherwise count.
I'll look into if there is a more elegant fix to this taking into
account your other suggestion.
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#newcode1403
src/heap/spaces.cc:1403: if (top_on_previous_step_) {
On 2015/07/23 10:58:54, Hannes Payer wrote:
Resetting here make the step still imprecise. We are loosing the
amount of
memory allocated right before resetting. I am fine with that, i.e. it
should not
be a problem for incremental marking. But you have to be aware of that
case.
Let's add a comment.
Acknowledged.
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 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.
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.
