Skip to content

Commit

Permalink
Apply ABSL_MUST_USE_RESULT for SystemRelease.
Browse files Browse the repository at this point in the history
PiperOrigin-RevId: 483475486
Change-Id: I883303ee37c9bc543a02eedfd3032270a3e05722
  • Loading branch information
ckennelly authored and copybara-github committed Oct 24, 2022
1 parent d5fe68e commit e33c7bc
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 12 deletions.
7 changes: 4 additions & 3 deletions tcmalloc/cpu_cache.h
Original file line number Diff line number Diff line change
Expand Up @@ -1590,9 +1590,10 @@ void CpuCache<Forwarder>::ResizeSlabIfNeeded() ABSL_NO_THREAD_SAFETY_ANALYSIS {
dynamic_slab_info_.madvise_failed_bytes.fetch_add(
info.old_slabs_size, std::memory_order_relaxed);
}
// TODO(b/122551676): Add a return value to SystemRelease and consume it
// here.
SystemRelease(info.old_slabs, info.old_slabs_size);
if (!SystemRelease(info.old_slabs, info.old_slabs_size)) {
dynamic_slab_info_.madvise_failed_bytes.fetch_add(
info.old_slabs_size, std::memory_order_relaxed);
}
forwarder_.ArenaReportNonresident(info.old_slabs_size, info.reused_bytes);
}

Expand Down
18 changes: 12 additions & 6 deletions tcmalloc/page_heap.cc
Original file line number Diff line number Diff line change
Expand Up @@ -370,11 +370,15 @@ Length PageHeap::ReleaseLastNormalSpan(SpanListPair* slist) {
pageheap_lock.Unlock();

const Length n = s->num_pages();
SystemRelease(s->start_address(), s->bytes_in_span());
bool success = SystemRelease(s->start_address(), s->bytes_in_span());

pageheap_lock.Lock();
stats_.free_bytes -= s->bytes_in_span();
s->set_location(Span::ON_RETURNED_FREELIST);
if (ABSL_PREDICT_TRUE(success)) {
stats_.free_bytes -= s->bytes_in_span();
s->set_location(Span::ON_RETURNED_FREELIST);
} else {
s->set_location(Span::ON_NORMAL_FREELIST);
}
MergeIntoFreeList(s); // Coalesces if possible.
return n;
}
Expand Down Expand Up @@ -446,7 +450,7 @@ bool PageHeap::GrowHeap(Length n) {
// Make sure pagemap has entries for all of the new pages.
// Plus ensure one before and one after so coalescing code
// does not need bounds-checking.
if (pagemap_->Ensure(p - Length(1), n + Length(2))) {
if (ABSL_PREDICT_TRUE(pagemap_->Ensure(p - Length(1), n + Length(2)))) {
// Pretend the new area is allocated and then return it to cause
// any necessary coalescing to occur.
Span* span = Span::New(p, n);
Expand All @@ -458,8 +462,10 @@ bool PageHeap::GrowHeap(Length n) {
} else {
// We could not allocate memory within the pagemap.
// Note the following leaks virtual memory, but at least it gets rid of
// the underlying physical memory.
SystemRelease(ptr, actual_size);
// the underlying physical memory. If SystemRelease fails, there's little
// we can do (we couldn't allocate for Ensure), but we have the consolation
// that the memory has not been touched (so it is likely not populated).
(void)SystemRelease(ptr, actual_size);
return false;
}
}
Expand Down
4 changes: 1 addition & 3 deletions tcmalloc/system-alloc.h
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,7 @@ int SystemReleaseErrors();
// be released, partial pages will not.)
//
// Returns true on success.
//
// TODO(b/122551676): Mark this ABSL_MUST_USE_RESULT.
bool SystemRelease(void* start, size_t length);
ABSL_MUST_USE_RESULT bool SystemRelease(void* start, size_t length);

// This call is the inverse of SystemRelease: the pages in this range
// are in use and should be faulted in. (In principle this is a
Expand Down

0 comments on commit e33c7bc

Please sign in to comment.