[compiler-rt] 97661b8 - Revert "sanitizer_common: optimize memory drain"
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Mon Jul 12 11:30:26 PDT 2021
Author: Nikita Popov
Date: 2021-07-12T20:28:28+02:00
New Revision: 97661b86345d2db084b147a0a36c63042eb1bc50
URL: https://github.com/llvm/llvm-project/commit/97661b86345d2db084b147a0a36c63042eb1bc50
DIFF: https://github.com/llvm/llvm-project/commit/97661b86345d2db084b147a0a36c63042eb1bc50.diff
LOG: Revert "sanitizer_common: optimize memory drain"
This reverts commit 072669521456a369409cf9db30739a3fac740173.
This causes the following build failure with gcc 10.3.0:
/home/nikic/llvm-project/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h:114:31: error: declaration of ‘typedef class __sanitizer::MemoryMapper<__sanitizer::SizeClassAllocator64<Params> > __sanitizer::SizeClassAllocator64<Params>::MemoryMapper’ changes meaning of ‘MemoryMapper’ [-fpermissive]
114 | typedef MemoryMapper<ThisT> MemoryMapper;
Added:
Modified:
compiler-rt/lib/sanitizer_common/sanitizer_allocator_local_cache.h
compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h
compiler-rt/lib/sanitizer_common/tests/sanitizer_allocator_test.cpp
Removed:
################################################################################
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_local_cache.h b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_local_cache.h
index 35d38e6f680d..108dfc231a22 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_local_cache.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_local_cache.h
@@ -17,7 +17,6 @@
template <class SizeClassAllocator>
struct SizeClassAllocator64LocalCache {
typedef SizeClassAllocator Allocator;
- typedef MemoryMapper<Allocator> MemoryMapper;
void Init(AllocatorGlobalStats *s) {
stats_.Init();
@@ -54,7 +53,7 @@ struct SizeClassAllocator64LocalCache {
PerClass *c = &per_class_[class_id];
InitCache(c);
if (UNLIKELY(c->count == c->max_count))
- Drain(c, allocator, class_id);
+ Drain(c, allocator, class_id, c->max_count / 2);
CompactPtrT chunk = allocator->PointerToCompactPtr(
allocator->GetRegionBeginBySizeClass(class_id),
reinterpret_cast<uptr>(p));
@@ -63,10 +62,10 @@ struct SizeClassAllocator64LocalCache {
}
void Drain(SizeClassAllocator *allocator) {
- MemoryMapper memory_mapper(*allocator);
for (uptr i = 1; i < kNumClasses; i++) {
PerClass *c = &per_class_[i];
- while (c->count > 0) Drain(&memory_mapper, c, allocator, i, c->count);
+ while (c->count > 0)
+ Drain(c, allocator, i, c->count);
}
}
@@ -107,18 +106,12 @@ struct SizeClassAllocator64LocalCache {
return true;
}
- NOINLINE void Drain(PerClass *c, SizeClassAllocator *allocator,
- uptr class_id) {
- MemoryMapper memory_mapper(*allocator);
- Drain(&memory_mapper, c, allocator, class_id, c->max_count / 2);
- }
-
- void Drain(MemoryMapper *memory_mapper, PerClass *c,
- SizeClassAllocator *allocator, uptr class_id, uptr count) {
+ NOINLINE void Drain(PerClass *c, SizeClassAllocator *allocator, uptr class_id,
+ uptr count) {
CHECK_GE(c->count, count);
const uptr first_idx_to_drain = c->count - count;
c->count -= count;
- allocator->ReturnToAllocator(memory_mapper, &stats_, class_id,
+ allocator->ReturnToAllocator(&stats_, class_id,
&c->chunks[first_idx_to_drain], count);
}
};
diff --git a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h
index dc26ec44d0ce..8bbed3c278c9 100644
--- a/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h
+++ b/compiler-rt/lib/sanitizer_common/sanitizer_allocator_primary64.h
@@ -42,60 +42,6 @@ struct SizeClassAllocator64FlagMasks { // Bit masks.
};
};
-template <typename Allocator>
-class MemoryMapper {
- public:
- typedef typename Allocator::CompactPtrT CompactPtrT;
-
- explicit MemoryMapper(const Allocator &allocator) : allocator_(allocator) {}
-
- ~MemoryMapper() {
- if (buffer_)
- UnmapOrDie(buffer_, buffer_size_);
- }
-
- bool GetAndResetStats(uptr &ranges, uptr &bytes) {
- ranges = released_ranges_count_;
- released_ranges_count_ = 0;
- bytes = released_bytes_;
- released_bytes_ = 0;
- return ranges != 0;
- }
-
- void *MapPackedCounterArrayBuffer(uptr buffer_size) {
- // TODO(alekseyshl): The idea to explore is to check if we have enough
- // space between num_freed_chunks*sizeof(CompactPtrT) and
- // mapped_free_array to fit buffer_size bytes and use that space instead
- // of mapping a temporary one.
- if (buffer_size_ < buffer_size) {
- if (buffer_)
- UnmapOrDie(buffer_, buffer_size_);
- buffer_ = MmapOrDieOnFatalError(buffer_size, "ReleaseToOSPageCounters");
- buffer_size_ = buffer_size;
- } else {
- internal_memset(buffer_, 0, buffer_size);
- }
- return buffer_;
- }
-
- // Releases [from, to) range of pages back to OS.
- void ReleasePageRangeToOS(CompactPtrT from, CompactPtrT to, uptr class_id) {
- const uptr region_base = allocator_.GetRegionBeginBySizeClass(class_id);
- const uptr from_page = allocator_.CompactPtrToPointer(region_base, from);
- const uptr to_page = allocator_.CompactPtrToPointer(region_base, to);
- ReleaseMemoryPagesToOS(from_page, to_page);
- released_ranges_count_++;
- released_bytes_ += to_page - from_page;
- }
-
- private:
- const Allocator &allocator_;
- uptr released_ranges_count_ = 0;
- uptr released_bytes_ = 0;
- void *buffer_ = nullptr;
- uptr buffer_size_ = 0;
-};
-
template <class Params>
class SizeClassAllocator64 {
public:
@@ -111,7 +57,6 @@ class SizeClassAllocator64 {
typedef SizeClassAllocator64<Params> ThisT;
typedef SizeClassAllocator64LocalCache<ThisT> AllocatorCache;
- typedef MemoryMapper<ThisT> MemoryMapper;
// When we know the size class (the region base) we can represent a pointer
// as a 4-byte integer (offset from the region start shifted right by 4).
@@ -175,10 +120,9 @@ class SizeClassAllocator64 {
}
void ForceReleaseToOS() {
- MemoryMapper memory_mapper(*this);
for (uptr class_id = 1; class_id < kNumClasses; class_id++) {
BlockingMutexLock l(&GetRegionInfo(class_id)->mutex);
- MaybeReleaseToOS(&memory_mapper, class_id, true /*force*/);
+ MaybeReleaseToOS(class_id, true /*force*/);
}
}
@@ -187,8 +131,7 @@ class SizeClassAllocator64 {
alignment <= SizeClassMap::kMaxSize;
}
- NOINLINE void ReturnToAllocator(MemoryMapper *memory_mapper,
- AllocatorStats *stat, uptr class_id,
+ NOINLINE void ReturnToAllocator(AllocatorStats *stat, uptr class_id,
const CompactPtrT *chunks, uptr n_chunks) {
RegionInfo *region = GetRegionInfo(class_id);
uptr region_beg = GetRegionBeginBySizeClass(class_id);
@@ -211,7 +154,7 @@ class SizeClassAllocator64 {
region->num_freed_chunks = new_num_freed_chunks;
region->stats.n_freed += n_chunks;
- MaybeReleaseToOS(memory_mapper, class_id, false /*force*/);
+ MaybeReleaseToOS(class_id, false /*force*/);
}
NOINLINE bool GetFromAllocator(AllocatorStats *stat, uptr class_id,
@@ -419,10 +362,10 @@ class SizeClassAllocator64 {
// For the performance sake, none of the accessors check the validity of the
// arguments, it is assumed that index is always in [0, n) range and the value
// is not incremented past max_value.
- template <typename MemoryMapper>
+ template<class MemoryMapperT>
class PackedCounterArray {
public:
- PackedCounterArray(u64 num_counters, u64 max_value, MemoryMapper *mapper)
+ PackedCounterArray(u64 num_counters, u64 max_value, MemoryMapperT *mapper)
: n(num_counters), memory_mapper(mapper) {
CHECK_GT(num_counters, 0);
CHECK_GT(max_value, 0);
@@ -446,6 +389,11 @@ class SizeClassAllocator64 {
buffer = reinterpret_cast<u64*>(
memory_mapper->MapPackedCounterArrayBuffer(buffer_size));
}
+ ~PackedCounterArray() {
+ if (buffer) {
+ memory_mapper->UnmapPackedCounterArrayBuffer(buffer, buffer_size);
+ }
+ }
bool IsAllocated() const {
return !!buffer;
@@ -482,21 +430,18 @@ class SizeClassAllocator64 {
u64 packing_ratio_log;
u64 bit_offset_mask;
- MemoryMapper *const memory_mapper;
+ MemoryMapperT* const memory_mapper;
u64 buffer_size;
u64* buffer;
};
- template <class MemoryMapperT>
+ template<class MemoryMapperT>
class FreePagesRangeTracker {
public:
- explicit FreePagesRangeTracker(MemoryMapperT *mapper, uptr class_id)
+ explicit FreePagesRangeTracker(MemoryMapperT* mapper)
: memory_mapper(mapper),
- class_id(class_id),
page_size_scaled_log(Log2(GetPageSizeCached() >> kCompactPtrScale)),
- in_the_range(false),
- current_page(0),
- current_range_start_page(0) {}
+ in_the_range(false), current_page(0), current_range_start_page(0) {}
void NextPage(bool freed) {
if (freed) {
@@ -518,14 +463,13 @@ class SizeClassAllocator64 {
void CloseOpenedRange() {
if (in_the_range) {
memory_mapper->ReleasePageRangeToOS(
- class_id, current_range_start_page << page_size_scaled_log,
+ current_range_start_page << page_size_scaled_log,
current_page << page_size_scaled_log);
in_the_range = false;
}
}
- MemoryMapperT *const memory_mapper;
- const uptr class_id;
+ MemoryMapperT* const memory_mapper;
const uptr page_size_scaled_log;
bool in_the_range;
uptr current_page;
@@ -536,12 +480,11 @@ class SizeClassAllocator64 {
// chunks only and returns these pages back to OS.
// allocated_pages_count is the total number of pages allocated for the
// current bucket.
- template <class MemoryMapper>
+ template<class MemoryMapperT>
static void ReleaseFreeMemoryToOS(CompactPtrT *free_array,
uptr free_array_count, uptr chunk_size,
uptr allocated_pages_count,
- MemoryMapper *memory_mapper,
- uptr class_id) {
+ MemoryMapperT *memory_mapper) {
const uptr page_size = GetPageSizeCached();
// Figure out the number of chunks per page and whether we can take a fast
@@ -577,8 +520,9 @@ class SizeClassAllocator64 {
UNREACHABLE("All chunk_size/page_size ratios must be handled.");
}
- PackedCounterArray<MemoryMapper> counters(
- allocated_pages_count, full_pages_chunk_count_max, memory_mapper);
+ PackedCounterArray<MemoryMapperT> counters(allocated_pages_count,
+ full_pages_chunk_count_max,
+ memory_mapper);
if (!counters.IsAllocated())
return;
@@ -603,7 +547,7 @@ class SizeClassAllocator64 {
// Iterate over pages detecting ranges of pages with chunk counters equal
// to the expected number of chunks for the particular page.
- FreePagesRangeTracker<MemoryMapper> range_tracker(memory_mapper, class_id);
+ FreePagesRangeTracker<MemoryMapperT> range_tracker(memory_mapper);
if (same_chunk_count_per_page) {
// Fast path, every page has the same number of chunks affecting it.
for (uptr i = 0; i < counters.GetCount(); i++)
@@ -642,7 +586,7 @@ class SizeClassAllocator64 {
}
private:
- friend class __sanitizer::MemoryMapper<ThisT>;
+ friend class MemoryMapper;
ReservedAddressRange address_range;
@@ -876,13 +820,57 @@ class SizeClassAllocator64 {
return true;
}
+ class MemoryMapper {
+ public:
+ MemoryMapper(const ThisT& base_allocator, uptr class_id)
+ : allocator(base_allocator),
+ region_base(base_allocator.GetRegionBeginBySizeClass(class_id)),
+ released_ranges_count(0),
+ released_bytes(0) {
+ }
+
+ uptr GetReleasedRangesCount() const {
+ return released_ranges_count;
+ }
+
+ uptr GetReleasedBytes() const {
+ return released_bytes;
+ }
+
+ void *MapPackedCounterArrayBuffer(uptr buffer_size) {
+ // TODO(alekseyshl): The idea to explore is to check if we have enough
+ // space between num_freed_chunks*sizeof(CompactPtrT) and
+ // mapped_free_array to fit buffer_size bytes and use that space instead
+ // of mapping a temporary one.
+ return MmapOrDieOnFatalError(buffer_size, "ReleaseToOSPageCounters");
+ }
+
+ void UnmapPackedCounterArrayBuffer(void *buffer, uptr buffer_size) {
+ UnmapOrDie(buffer, buffer_size);
+ }
+
+ // Releases [from, to) range of pages back to OS.
+ void ReleasePageRangeToOS(CompactPtrT from, CompactPtrT to) {
+ const uptr from_page = allocator.CompactPtrToPointer(region_base, from);
+ const uptr to_page = allocator.CompactPtrToPointer(region_base, to);
+ ReleaseMemoryPagesToOS(from_page, to_page);
+ released_ranges_count++;
+ released_bytes += to_page - from_page;
+ }
+
+ private:
+ const ThisT& allocator;
+ const uptr region_base;
+ uptr released_ranges_count;
+ uptr released_bytes;
+ };
+
// Attempts to release RAM occupied by freed chunks back to OS. The region is
// expected to be locked.
//
// TODO(morehouse): Support a callback on memory release so HWASan can release
// aliases as well.
- void MaybeReleaseToOS(MemoryMapper *memory_mapper, uptr class_id,
- bool force) {
+ void MaybeReleaseToOS(uptr class_id, bool force) {
RegionInfo *region = GetRegionInfo(class_id);
const uptr chunk_size = ClassIdToSize(class_id);
const uptr page_size = GetPageSizeCached();
@@ -906,16 +894,17 @@ class SizeClassAllocator64 {
}
}
- ReleaseFreeMemoryToOS(
+ MemoryMapper memory_mapper(*this, class_id);
+
+ ReleaseFreeMemoryToOS<MemoryMapper>(
GetFreeArray(GetRegionBeginBySizeClass(class_id)), n, chunk_size,
- RoundUpTo(region->allocated_user, page_size) / page_size, memory_mapper,
- class_id);
+ RoundUpTo(region->allocated_user, page_size) / page_size,
+ &memory_mapper);
- uptr ranges, bytes;
- if (memory_mapper->GetAndResetStats(ranges, bytes)) {
+ if (memory_mapper.GetReleasedRangesCount() > 0) {
region->rtoi.n_freed_at_last_release = region->stats.n_freed;
- region->rtoi.num_releases += ranges;
- region->rtoi.last_released_bytes = bytes;
+ region->rtoi.num_releases += memory_mapper.GetReleasedRangesCount();
+ region->rtoi.last_released_bytes = memory_mapper.GetReleasedBytes();
}
region->rtoi.last_release_at_ns = MonotonicNanoTime();
}
diff --git a/compiler-rt/lib/sanitizer_common/tests/sanitizer_allocator_test.cpp b/compiler-rt/lib/sanitizer_common/tests/sanitizer_allocator_test.cpp
index a5076da5aa18..58f1ef404ba6 100644
--- a/compiler-rt/lib/sanitizer_common/tests/sanitizer_allocator_test.cpp
+++ b/compiler-rt/lib/sanitizer_common/tests/sanitizer_allocator_test.cpp
@@ -1243,7 +1243,7 @@ class RangeRecorder {
Log2(GetPageSizeCached() >> Allocator64::kCompactPtrScale)),
last_page_reported(0) {}
- void ReleasePageRangeToOS(u32 class_id, u32 from, u32 to) {
+ void ReleasePageRangeToOS(u32 from, u32 to) {
from >>= page_size_scaled_log;
to >>= page_size_scaled_log;
ASSERT_LT(from, to);
@@ -1253,7 +1253,6 @@ class RangeRecorder {
reported_pages.append(to - from, 'x');
last_page_reported = to;
}
-
private:
const uptr page_size_scaled_log;
u32 last_page_reported;
@@ -1283,7 +1282,7 @@ TEST(SanitizerCommon, SizeClassAllocator64FreePagesRangeTracker) {
for (auto test_case : test_cases) {
RangeRecorder range_recorder;
- RangeTracker tracker(&range_recorder, 1);
+ RangeTracker tracker(&range_recorder);
for (int i = 0; test_case[i] != 0; i++)
tracker.NextPage(test_case[i] == 'x');
tracker.Done();
@@ -1309,7 +1308,7 @@ class ReleasedPagesTrackingMemoryMapper {
free(buffer);
}
- void ReleasePageRangeToOS(u32 class_id, u32 from, u32 to) {
+ void ReleasePageRangeToOS(u32 from, u32 to) {
uptr page_size_scaled =
GetPageSizeCached() >> Allocator64::kCompactPtrScale;
for (u32 i = from; i < to; i += page_size_scaled)
@@ -1353,7 +1352,7 @@ void TestReleaseFreeMemoryToOS() {
Allocator::ReleaseFreeMemoryToOS(&free_array[0], free_array.size(),
chunk_size, kAllocatedPagesCount,
- &memory_mapper, class_id);
+ &memory_mapper);
// Verify that there are no released pages touched by used chunks and all
// ranges of free chunks big enough to contain the entire memory pages had
More information about the llvm-commits
mailing list