Commit 95249bca authored by Ali Ijaz Sheikh's avatar Ali Ijaz Sheikh Committed by Commit Bot

[heap] allocation step should before limit update

Do a step before selecting the limit for the next step. However, as seen
on crbug.com/795323, while this fix makes us more precise in our
accounting, we do ending up seeing steps more frequently. This ends up
invoking the idle scavenger more frequently. To compensate, we adjust
the idle scavenger step size.

Bug: 
Change-Id: I7bc2b1785a564dee27aa3ce6a5a196efe9eb6283
Reviewed-on: https://chromium-review.googlesource.com/838440
Commit-Queue: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-by: 's avatarUlan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#50816}
parent c13fd598
......@@ -64,7 +64,7 @@ class V8_EXPORT_PRIVATE ScavengeJob {
static const int kAverageIdleTimeMs = 5;
// The number of bytes to be allocated in new space before the next idle
// task is posted.
static const size_t kBytesAllocatedBeforeNextIdleTask = 512 * KB;
static const size_t kBytesAllocatedBeforeNextIdleTask = 1024 * KB;
// The minimum size of allocated new space objects to trigger a scavenge.
static const size_t kMinAllocationLimit = 512 * KB;
// The allocation limit cannot exceed this fraction of the new space capacity.
......
......@@ -2081,16 +2081,13 @@ LocalAllocationBuffer& LocalAllocationBuffer::operator=(
void NewSpace::UpdateLinearAllocationArea() {
Address old_top = top();
Address new_top = to_space_.page_low();
InlineAllocationStep(old_top, new_top, nullptr, 0);
MemoryChunk::UpdateHighWaterMark(allocation_info_.top());
allocation_info_.Reset(new_top, to_space_.page_high());
original_top_.SetValue(top());
original_limit_.SetValue(limit());
UpdateInlineAllocationLimit(0);
// TODO(ofrobots): It would be more correct to do a step before setting the
// limit on the new allocation area. However, fixing this causes a regression
// due to the idle scavenger getting pinged too frequently. crbug.com/795323.
InlineAllocationStep(old_top, new_top, nullptr, 0);
DCHECK_SEMISPACE_ALLOCATION_INFO(allocation_info_, to_space_);
}
......
......@@ -6,6 +6,7 @@
#include "src/globals.h"
#include "src/heap/scavenge-job.h"
#include "src/utils.h"
#include "testing/gtest/include/gtest/gtest.h"
namespace v8 {
......@@ -31,6 +32,8 @@ TEST(ScavengeJob, AllocationLimitUnknownScavengeSpeed) {
size_t expected_size = ScavengeJob::kInitialScavengeSpeedInBytesPerMs *
ScavengeJob::kAverageIdleTimeMs -
ScavengeJob::kBytesAllocatedBeforeNextIdleTask;
expected_size = Max(expected_size, ScavengeJob::kMinAllocationLimit);
EXPECT_FALSE(ScavengeJob::ReachedIdleAllocationLimit(0, expected_size - 1,
kNewSpaceCapacity));
EXPECT_TRUE(ScavengeJob::ReachedIdleAllocationLimit(0, expected_size,
......
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