From 1c5e511dc20fc7ff5f5b29ab4157a2143a3d1ede Mon Sep 17 00:00:00 2001 From: ofrobots Date: Thu, 24 Sep 2015 11:22:09 -0700 Subject: [PATCH] [heap] refactor inline allocation step code Once I improved byte accounting done for incremental mark in [1], there is some code duplication that becomes apparent. This commit refactors the duplicated code into a private method on NewSpace. This also makes it easy to add new consumers of inline allocation steps in the future. [1] https://codereview.chromium.org/1274453002/ R=hpayer@chromium.org BUG= Review URL: https://codereview.chromium.org/1351983002 Cr-Commit-Position: refs/heads/master@{#30921} --- src/heap/spaces.cc | 33 +++++++++++++-------------------- src/heap/spaces.h | 9 +++++++++ 2 files changed, 22 insertions(+), 20 deletions(-) diff --git a/src/heap/spaces.cc b/src/heap/spaces.cc index cd9d627..402a6c0 100644 --- a/src/heap/spaces.cc +++ b/src/heap/spaces.cc @@ -1400,12 +1400,7 @@ void NewSpace::ResetAllocationInfo() { while (it.has_next()) { Bitmap::Clear(it.next()); } - if (top_on_previous_step_) { - int bytes_allocated = static_cast(old_top - top_on_previous_step_); - heap()->incremental_marking()->Step(bytes_allocated, - IncrementalMarking::GC_VIA_STACK_GUARD); - top_on_previous_step_ = allocation_info_.top(); - } + InlineAllocationStep(old_top, allocation_info_.top()); } @@ -1484,13 +1479,7 @@ bool NewSpace::EnsureAllocation(int size_in_bytes, return false; } - if (top_on_previous_step_) { - // Do a step for the bytes allocated on the last page. - int bytes_allocated = static_cast(old_top - top_on_previous_step_); - heap()->incremental_marking()->Step( - bytes_allocated, IncrementalMarking::GC_VIA_STACK_GUARD); - top_on_previous_step_ = allocation_info_.top(); - } + InlineAllocationStep(old_top, allocation_info_.top()); old_top = allocation_info_.top(); high = to_space_.page_high(); @@ -1504,19 +1493,23 @@ bool NewSpace::EnsureAllocation(int size_in_bytes, // Either the limit has been lowered because linear allocation was disabled // or because incremental marking wants to get a chance to do a step. Set // the new limit accordingly. - if (top_on_previous_step_) { - Address new_top = old_top + aligned_size_in_bytes; - int bytes_allocated = static_cast(new_top - top_on_previous_step_); - heap()->incremental_marking()->Step( - bytes_allocated, IncrementalMarking::GC_VIA_STACK_GUARD); - top_on_previous_step_ = new_top; - } + Address new_top = old_top + aligned_size_in_bytes; + InlineAllocationStep(new_top, new_top); UpdateInlineAllocationLimit(aligned_size_in_bytes); } return true; } +void NewSpace::InlineAllocationStep(Address top, Address new_top) { + if (top_on_previous_step_) { + int bytes_allocated = static_cast(top - top_on_previous_step_); + heap()->incremental_marking()->Step(bytes_allocated, + IncrementalMarking::GC_VIA_STACK_GUARD); + top_on_previous_step_ = new_top; + } +} + #ifdef VERIFY_HEAP // We do not use the SemiSpaceIterator because verification doesn't assume // that it works (it depends on the invariants we are checking). diff --git a/src/heap/spaces.h b/src/heap/spaces.h index 029e5c6..2cea066 100644 --- a/src/heap/spaces.h +++ b/src/heap/spaces.h @@ -2721,6 +2721,15 @@ class NewSpace : public Space { bool EnsureAllocation(int size_in_bytes, AllocationAlignment alignment); + // If we are doing inline allocation in steps, this method performs the 'step' + // operation. Right now incremental marking is the only consumer of inline + // allocation steps. top is the memory address of the bump pointer at the last + // inline allocation (i.e. it determines the numbers of bytes actually + // allocated since the last step.) new_top is the address of the bump pointer + // where the next byte is going to be allocated from. top and new_top may be + // different when we cross a page boundary or reset the space. + void InlineAllocationStep(Address top, Address new_top); + friend class SemiSpaceIterator; }; -- 2.7.4