Commit 85ec7854 authored by Chris Palmer's avatar Chris Palmer Committed by Commit Bot

Revert "[Partition Alloc] Don't redundantly call madvise redundantly."

This reverts commit be7b68a7.

Reason for revert: Lost the crucial CHECK(!ret) in a merge conflict, and re-adding it causes macOS to fail (EINVAL, invalid address). That requires further investigation.

Original change's description:
> [Partition Alloc] Don't redundantly call madvise redundantly.
> 
> When
> 
>   #ifndef MADV_FREE
>   #define MADV_FREE MADV_DONTNEED
>   #endif
> 
> we would call madvise(..., MADV_FREE); and then, if that failed, call
> madvise(..., MADV_DONTNEED). This would result in pointlessly calling
> madvise(..., MADV_DONTNEED) and then trying again instead of CHECKing
> immediately.
> 
> This is more of a readability refactor than a performance-relevant change,
> obviously.
> 
> Thanks to ajwong for noticing this!
> 
> BUG=766882,755284
> 
> Change-Id: If8cbe14f38dfe2bd126bc24f79e670c45b29c8d5
> Reviewed-on: https://chromium-review.googlesource.com/754038
> Reviewed-by: Primiano Tucci <primiano@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Commit-Queue: Chris Palmer <palmer@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#514635}

TBR=ajwong@chromium.org,palmer@chromium.org,primiano@chromium.org,haraken@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug: 766882, 755284
Change-Id: I2b71c2f6773c38109d17c151e5a81aa50bc46b10
Reviewed-on: https://chromium-review.googlesource.com/759058Reviewed-by: default avatarChris Palmer <palmer@chromium.org>
Reviewed-by: default avatarKentaro Hara <haraken@chromium.org>
Reviewed-by: default avatarAlbert J. Wong <ajwong@chromium.org>
Commit-Queue: Chris Palmer <palmer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#515091}
parent 8bd9576f
......@@ -24,6 +24,10 @@
#include <errno.h>
#include <sys/mman.h>
#ifndef MADV_FREE
#define MADV_FREE MADV_DONTNEED
#endif
#ifndef MAP_ANONYMOUS
#define MAP_ANONYMOUS MAP_ANON
#endif
......@@ -288,7 +292,6 @@ bool SetSystemPagesAccess(void* address,
void DecommitSystemPages(void* address, size_t length) {
DCHECK_EQ(0UL, length & kSystemPageOffsetMask);
#if defined(OS_POSIX)
// In POSIX, there is no decommit concept. Discarding is an effective way of
// implementing the Windows semantics where the OS is allowed to not swap the
......@@ -321,27 +324,22 @@ bool RecommitSystemPages(void* address,
void DiscardSystemPages(void* address, size_t length) {
DCHECK_EQ(0UL, length & kSystemPageOffsetMask);
#if defined(OS_POSIX)
int ret = -1;
#if defined(OS_MACOSX)
// On macOS, MADV_FREE_REUSABLE has comparable behavior to MADV_FREE, but also
// marks the pages with the reusable bit, which allows both Activity Monitor
// and memory-infra to correctly track the pages.
ret = madvise(address, length, MADV_FREE_REUSABLE);
int ret = madvise(address, length, MADV_FREE_REUSABLE);
#else
#if defined(MADV_FREE)
ret = madvise(address, length, MADV_FREE);
int ret = madvise(address, length, MADV_FREE);
#endif
if (ret != 0 && errno == EINVAL) {
// MADV_FREE only works on Linux 4.5+. If the request failed, retry with the
// older MADV_DONTNEED. (Note that MADV_FREE being defined at compile time
// doesn't imply runtime support.)
// MADV_FREE only works on Linux 4.5+ . If request failed,
// retry with older MADV_DONTNEED . Note that MADV_FREE
// being defined at compile time doesn't imply runtime support.
ret = madvise(address, length, MADV_DONTNEED);
}
#else
ret = madvise(address, length, MADV_DONTNEED);
#endif // MADV_FREE
#endif // OS_MACOSX
CHECK(!ret);
#else
// On Windows discarded pages are not returned to the system immediately and
// not guaranteed to be zeroed when returned to the application.
......@@ -365,7 +363,7 @@ void DiscardSystemPages(void* address, size_t length) {
void* ret = VirtualAlloc(address, length, MEM_RESET, PAGE_READWRITE);
CHECK(ret);
}
#endif // OS_POSIX
#endif
}
bool ReserveAddressSpace(size_t 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