Commit 4623b822 authored by Ali Ijaz Sheikh's avatar Ali Ijaz Sheikh Committed by Commit Bot

[heap] replace DisableInlineAllocationSteps with PauseAllocationObservers

* DisableInlineAllocationSteps was a blunt hammer added to work around
  tests that needed to avoid artificially lower limits imposed by
  observers. PauseAllocationObserversScope can properly disable step on
  a temporary basis.
* Modify tests. Remove DisableInlineAllocationSteps.

This exposed a bug in allocation observers: we were not doing a step
when a fresh page is added.

Fix this by moving the step into UpdateAllocationInfo. We should be
doing a step (and keeping top_on_previous_step_ consistent) whenever
we move move top(). UpdateAllocationInfo is the correct place for this
rather than the callers of UpdateAllocationInfo.

Bug: 
Change-Id: I2edc238dc2e73bf9a2e9738c2a9b50efcac5cbf0
Reviewed-on: https://chromium-review.googlesource.com/821052
Commit-Queue: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50052}
parent 10d9c314
......@@ -2056,8 +2056,11 @@ LocalAllocationBuffer& LocalAllocationBuffer::operator=(
void NewSpace::UpdateAllocationInfo() {
Address new_top = to_space_.page_low();
InlineAllocationStep(top(), new_top, nullptr, 0);
MemoryChunk::UpdateHighWaterMark(allocation_info_.top());
allocation_info_.Reset(to_space_.page_low(), to_space_.page_high());
allocation_info_.Reset(new_top, to_space_.page_high());
original_top_.SetValue(top());
original_limit_.SetValue(limit());
UpdateInlineAllocationLimit(0);
......@@ -2066,7 +2069,6 @@ void NewSpace::UpdateAllocationInfo() {
void NewSpace::ResetAllocationInfo() {
Address old_top = allocation_info_.top();
to_space_.Reset();
UpdateAllocationInfo();
// Clear all mark-bits in the to-space.
......@@ -2077,7 +2079,6 @@ void NewSpace::ResetAllocationInfo() {
// Concurrent marking may have local live bytes for this page.
heap()->concurrent_marking()->ClearLiveness(p);
}
InlineAllocationStep(old_top, allocation_info_.top(), nullptr, 0);
}
......@@ -2138,8 +2139,6 @@ bool NewSpace::EnsureAllocation(int size_in_bytes,
return false;
}
InlineAllocationStep(old_top, allocation_info_.top(), nullptr, 0);
old_top = allocation_info_.top();
high = to_space_.page_high();
filler_size = Heap::GetFillToAlign(old_top, alignment);
......
......@@ -2681,11 +2681,6 @@ class NewSpace : public SpaceWithLinearArea {
// it in steps to guarantee that the observers are notified periodically.
void UpdateInlineAllocationLimit(int size_in_bytes);
void DisableInlineAllocationSteps() {
top_on_previous_step_ = 0;
UpdateInlineAllocationLimit(0);
}
// Get the extent of the inactive semispace (for use as a marking stack,
// or to zap it). Notice: space-addresses are not necessarily on the
// same page, so FromSpaceStart() might be above FromSpaceEnd().
......
......@@ -72,7 +72,6 @@ std::vector<Handle<FixedArray>> CreatePadding(Heap* heap, int padding_size,
int overall_free_memory = static_cast<int>(heap->old_space()->Available());
CHECK(padding_size <= overall_free_memory || overall_free_memory == 0);
} else {
heap->new_space()->DisableInlineAllocationSteps();
int overall_free_memory =
static_cast<int>(*heap->new_space()->allocation_limit_address() -
*heap->new_space()->allocation_top_address());
......@@ -105,7 +104,7 @@ std::vector<Handle<FixedArray>> CreatePadding(Heap* heap, int padding_size,
void AllocateAllButNBytes(v8::internal::NewSpace* space, int extra_bytes,
std::vector<Handle<FixedArray>>* out_handles) {
space->DisableInlineAllocationSteps();
PauseAllocationObserversScope pause_observers(space->heap());
int space_remaining = static_cast<int>(*space->allocation_limit_address() -
*space->allocation_top_address());
CHECK(space_remaining >= extra_bytes);
......@@ -124,7 +123,7 @@ void FillCurrentPage(v8::internal::NewSpace* space,
bool FillUpOnePage(v8::internal::NewSpace* space,
std::vector<Handle<FixedArray>>* out_handles) {
space->DisableInlineAllocationSteps();
PauseAllocationObserversScope pause_observers(space->heap());
int space_remaining = static_cast<int>(*space->allocation_limit_address() -
*space->allocation_top_address());
if (space_remaining == 0) return false;
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment