Commit fd8ea126 authored by Bartek Nowierski's avatar Bartek Nowierski Committed by Chromium LUCI CQ

[PA] Disable lazy commit on non-Windows platforms

crrev.com/c/2563385 showed a lot of good PMF improvements on Windows,
but came at a cost of regressions on non-Windows platforms:
https://chromeperf.appspot.com/group_report?rev=833127

This is possibly related to the fact that my CL took out the
optimization to not change permissions on recommit, out of necessity.
This affects only non-Windows platforms, because Windows doesn't have
this optimization to begin with. Therefore this CL disables it
everywhere except Windows.

Bug: 1155066, 1155062, 1155172, 1155172

Change-Id: Ie76801e58c8f65aa66cd9d37ad76020f0009149d
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2569143
Commit-Queue: Bartek Nowierski <bartekn@chromium.org>
Commit-Queue: Kentaro Hara <haraken@chromium.org>
Auto-Submit: Bartek Nowierski <bartekn@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#833633}
parent f3d6ac24
......@@ -1432,7 +1432,12 @@ TEST_F(PartitionAllocTest, FreeCache) {
allocator.root()->AdjustPointerForExtrasSubtract(ptr));
EXPECT_EQ(nullptr, bucket->empty_slot_spans_head);
EXPECT_EQ(1, slot_span->num_allocated_slots);
#if defined(OS_WIN)
// Windows uses lazy commit, thus committing only needed pages.
size_t expected_committed_size = SystemPageSize();
#else
size_t expected_committed_size = PartitionPageSize();
#endif
EXPECT_EQ(expected_committed_size,
allocator.root()->get_total_size_of_committed_pages());
allocator.root()->Free(ptr);
......@@ -1446,7 +1451,15 @@ TEST_F(PartitionAllocTest, FreeCache) {
EXPECT_FALSE(slot_span->freelist_head);
EXPECT_EQ(-1, slot_span->empty_cache_index);
EXPECT_EQ(0, slot_span->num_allocated_slots);
#if defined(OS_WIN)
size_t expected_size = SystemPageSize();
#else
PartitionBucket<base::internal::ThreadSafe>* cycle_free_cache_bucket =
&allocator.root()->buckets[test_bucket_index_];
size_t expected_size =
cycle_free_cache_bucket->num_system_pages_per_slot_span *
SystemPageSize();
#endif
EXPECT_EQ(expected_size,
allocator.root()->get_total_size_of_committed_pages());
......@@ -2565,7 +2578,12 @@ TEST_F(PartitionAllocTest, MAYBE_Bookkeeping) {
// A full slot span of size 1 partition page is committed.
void* ptr = root.Alloc(small_size - kExtraAllocSize, type_name);
#if defined(OS_WIN)
// Windows uses lazy commit, thus committing only needed pages.
size_t expected_committed_size = SystemPageSize();
#else
size_t expected_committed_size = PartitionPageSize();
#endif
size_t expected_super_pages_size = kSuperPageSize;
EXPECT_EQ(expected_committed_size, root.total_size_of_committed_pages);
EXPECT_EQ(expected_super_pages_size, root.total_size_of_super_pages);
......@@ -2587,7 +2605,11 @@ TEST_F(PartitionAllocTest, MAYBE_Bookkeeping) {
// Allocating another size commits another slot span.
ptr = root.Alloc(2 * small_size - kExtraAllocSize, type_name);
#if defined(OS_WIN)
expected_committed_size += SystemPageSize();
#else
expected_committed_size += PartitionPageSize();
#endif
EXPECT_EQ(expected_committed_size, root.total_size_of_committed_pages);
EXPECT_EQ(expected_super_pages_size, root.total_size_of_super_pages);
......
......@@ -217,6 +217,14 @@ ALWAYS_INLINE void* PartitionBucket<thread_safe>::AllocNewSlotSpan(
char* ret = root->next_partition_page;
root->next_partition_page += slot_span_reserved_size;
#if !defined(OS_WIN)
// System pages in the super page come in a decommited state. Commit them
// before vending them back.
// Windows uses lazy commit. Pages will be committed when provisioning slots,
// in ProvisionMoreSlotsAndAllocOne().
root->RecommitSystemPagesForData(ret, slot_span_committed_size,
PageUpdatePermissions);
#endif
// Double check that we had enough space in the super page for the new slot
// span.
PA_DCHECK(root->next_partition_page <= root->next_partition_page_end);
......@@ -379,11 +387,15 @@ ALWAYS_INLINE char* PartitionBucket<thread_safe>::ProvisionMoreSlotsAndAllocOne(
// to the page start and |next_slot| doesn't, thus only the latter gets
// rounded up.
PA_DCHECK(commit_end > commit_start);
// System pages in the slot span come in an initially decommitted state.
// Can't use PageKeepPermissionsIfPossible, because we have no knowledge
// which pages have been committed before.
#if defined(OS_WIN)
// Windows uses lazy commit, meaning system pages in the slot span come in an
// initially decommitted state. Commit them here.
// Note, we can't use PageKeepPermissionsIfPossible, because we have no
// knowledge which pages have been committed before (it doesn't matter on
// Windows anyway).
root->RecommitSystemPagesForData(commit_start, commit_end - commit_start,
PageUpdatePermissions);
#endif
// The slot being returned is considered allocated, and no longer
// unprovisioned.
......@@ -560,6 +572,14 @@ void* PartitionBucket<thread_safe>::SlowPathAlloc(
PA_DCHECK(new_slot_span->bucket == this);
PA_DCHECK(new_slot_span->is_decommitted());
decommitted_slot_spans_head = new_slot_span->next_slot_span;
#if !defined(OS_WIN)
// Windows uses lazy commit. Pages will be recommitted when provisioning
// slots, in ProvisionMoreSlotsAndAllocOne().
void* addr = SlotSpanMetadata<thread_safe>::ToPointer(new_slot_span);
root->RecommitSystemPagesForData(
addr, new_slot_span->bucket->get_bytes_per_span(),
PageKeepPermissionsIfPossible);
#endif
new_slot_span->Reset();
*is_already_zeroed = kDecommittedPagesAreAlwaysZeroed;
}
......
......@@ -169,7 +169,13 @@ void SlotSpanMetadata<thread_safe>::Decommit(PartitionRoot<thread_safe>* root) {
PA_DCHECK(is_empty());
PA_DCHECK(!bucket->is_direct_mapped());
void* addr = SlotSpanMetadata::ToPointer(this);
size_t size_to_decommit = bits::Align(GetProvisionedSize(), SystemPageSize());
size_t size_to_decommit =
#if defined(OS_WIN)
// Windows uses lazy commit, thus only provisioned slots are committed.
bits::Align(GetProvisionedSize(), SystemPageSize());
#else
bucket->get_bytes_per_span();
#endif
// Not decommitted slot span must've had at least 1 allocation.
PA_DCHECK(size_to_decommit > 0);
root->DecommitSystemPagesForData(addr, size_to_decommit,
......
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