Revert "[vm, service] Adjust GetProcessMemoryUsage and scavenger stats to avoid races."

This reverts commit 782611f29a.

Reason for revert: divide by zero in ExpectedGarbageFraction

Original change's description:
> [vm, service] Adjust GetProcessMemoryUsage and scavenger stats to avoid races.
>
> TEST=tsan
> Bug: https://github.com/dart-lang/sdk/issues/52862
> Change-Id: I90d31ff28e2517ddd2b1e7b387fa38152144f45c
> Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/324766
> Reviewed-by: Ben Konyi <bkonyi@google.com>
> Commit-Queue: Ryan Macnak <rmacnak@google.com>

Bug: https://github.com/dart-lang/sdk/issues/52862
Change-Id: I8e0502087f64fa470000690a6d31e7be6b01b3fe
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/326865
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Commit-Queue: Ryan Macnak <rmacnak@google.com>
Reviewed-by: Ben Konyi <bkonyi@google.com>
This commit is contained in:
Ryan Macnak 2023-09-20 18:39:25 +00:00 committed by Commit Queue
parent c811560d64
commit 1de2110b6a
5 changed files with 24 additions and 35 deletions

View file

@ -1147,7 +1147,7 @@ void Heap::PrintStatsToTimeline(TimelineEventScope* event, GCReason reason) {
Heap::Space Heap::SpaceForExternal(intptr_t size) const {
// If 'size' would be a significant fraction of new space, then use old.
const int kExtNewRatio = 16;
if (size > (new_space_.ThresholdInWords() * kWordSize) / kExtNewRatio) {
if (size > (CapacityInWords(Heap::kNew) * kWordSize) / kExtNewRatio) {
return Heap::kOld;
} else {
return Heap::kNew;

View file

@ -204,13 +204,13 @@ class PageSpace {
void UpdateMaxCapacityLocked();
void UpdateMaxUsed();
intptr_t ExternalInWords() const { return usage_.external_in_words; }
int64_t ExternalInWords() const { return usage_.external_in_words; }
SpaceUsage GetCurrentUsage() const {
MutexLocker ml(&pages_lock_);
return usage_;
}
intptr_t ImageInWords() const {
intptr_t size = 0;
int64_t ImageInWords() const {
int64_t size = 0;
MutexLocker ml(&pages_lock_);
for (Page* page = image_pages_; page != nullptr; page = page->next()) {
size += page->memory_->size();

View file

@ -731,8 +731,8 @@ class ParallelScavengerTask : public ThreadPool::Task {
DISALLOW_COPY_AND_ASSIGN(ParallelScavengerTask);
};
SemiSpace::SemiSpace(intptr_t gc_threshold_in_words)
: gc_threshold_in_words_(gc_threshold_in_words) {}
SemiSpace::SemiSpace(intptr_t max_capacity_in_words)
: max_capacity_in_words_(max_capacity_in_words), head_(nullptr) {}
SemiSpace::~SemiSpace() {
Page* page = head_;
@ -744,7 +744,7 @@ SemiSpace::~SemiSpace() {
}
Page* SemiSpace::TryAllocatePageLocked(bool link) {
if (capacity_in_words_ >= gc_threshold_in_words_) {
if (capacity_in_words_ >= max_capacity_in_words_) {
return nullptr; // Full.
}
Page* page = Page::Allocate(kPageSize, Page::kNew);
@ -1034,16 +1034,15 @@ SemiSpace* Scavenger::Prologue(GCReason reason) {
// new remembered set.
blocks_ = heap_->isolate_group()->store_buffer()->TakeBlocks();
UpdateMaxHeapCapacity();
// Flip the two semi-spaces so that to_ is always the space for allocating
// objects.
SemiSpace* from;
{
MutexLocker ml(&space_lock_);
from = to_;
to_ = new SemiSpace(NewSizeInWords(from->gc_threshold_in_words(), reason));
to_ = new SemiSpace(NewSizeInWords(from->max_capacity_in_words(), reason));
}
UpdateMaxHeapCapacity();
return from;
}
@ -1107,7 +1106,7 @@ void Scavenger::Epilogue(SemiSpace* from) {
// Even if the scavenge speed is very high, make sure we start considering
// idle scavenges before new space is full to avoid requiring a scavenge in
// the middle of a frame.
intptr_t upper_bound = 8 * ThresholdInWords() / 10;
intptr_t upper_bound = 8 * CapacityInWords() / 10;
if (idle_scavenge_threshold_in_words_ > upper_bound) {
idle_scavenge_threshold_in_words_ = upper_bound;
}
@ -1354,7 +1353,7 @@ void Scavenger::UpdateMaxHeapCapacity() {
auto isolate_group = heap_->isolate_group();
ASSERT(isolate_group != nullptr);
isolate_group->GetHeapNewCapacityMaxMetric()->SetValue(
to_->capacity_in_words() * kWordSize);
to_->max_capacity_in_words() * kWordSize);
}
void Scavenger::UpdateMaxHeapUsage() {
@ -1695,7 +1694,7 @@ void Scavenger::Scavenge(Thread* thread, GCType type, GCReason reason) {
heap_->stats_.state_ = Heap::kSecondScavenge;
}
}
if ((ThresholdInWords() - UsedInWords()) < KBInWords) {
if ((CapacityInWords() - UsedInWords()) < KBInWords) {
// Don't scavenge again until the next old-space GC has occurred. Prevents
// performing one scavenge per allocation as the heap limit is approached.
heap_->assume_scavenge_will_fail_ = true;

View file

@ -32,7 +32,7 @@ class ScavengerVisitorBase;
class SemiSpace {
public:
explicit SemiSpace(intptr_t gc_threshold_in_words);
explicit SemiSpace(intptr_t max_capacity_in_words);
~SemiSpace();
Page* TryAllocatePageLocked(bool link);
@ -48,19 +48,18 @@ class SemiSpace {
return size >> kWordSizeLog2;
}
intptr_t capacity_in_words() const { return capacity_in_words_; }
intptr_t gc_threshold_in_words() const { return gc_threshold_in_words_; }
intptr_t max_capacity_in_words() const { return max_capacity_in_words_; }
Page* head() const { return head_; }
void AddList(Page* head, Page* tail);
private:
// Size of Pages in this semi-space.
// Size of NewPages in this semi-space.
intptr_t capacity_in_words_ = 0;
// Size of Pages before we trigger a scavenge. Compare old-space's
// hard_gc_threshold_in_words_.
intptr_t gc_threshold_in_words_;
// Size of NewPages before we trigger a scavenge.
intptr_t max_capacity_in_words_;
Page* head_ = nullptr;
Page* tail_ = nullptr;
@ -157,10 +156,7 @@ class Scavenger {
MutexLocker ml(&space_lock_);
return to_->used_in_words();
}
intptr_t CapacityInWords() const {
MutexLocker ml(&space_lock_);
return to_->capacity_in_words();
}
intptr_t CapacityInWords() const { return to_->max_capacity_in_words(); }
intptr_t ExternalInWords() const { return external_size_ >> kWordSizeLog2; }
SpaceUsage GetCurrentUsage() const {
SpaceUsage usage;
@ -169,7 +165,6 @@ class Scavenger {
usage.external_in_words = ExternalInWords();
return usage;
}
intptr_t ThresholdInWords() const { return to_->gc_threshold_in_words(); }
void VisitObjects(ObjectVisitor* visitor) const;
void VisitObjectPointers(ObjectPointerVisitor* visitor) const;
@ -217,6 +212,8 @@ class Scavenger {
ASSERT(external_size_ >= 0);
}
int64_t FreeSpaceInWords(Isolate* isolate) const;
// The maximum number of Dart mutator threads we allow to execute at the same
// time.
static intptr_t MaxMutatorThreadCount() {

View file

@ -4760,20 +4760,13 @@ static intptr_t GetProcessMemoryUsageHelper(JSONStream* js) {
IsolateGroup::ForEach([&vm_children,
&vm_size](IsolateGroup* isolate_group) {
// Note: new_space()->CapacityInWords() includes memory that hasn't been
// allocated from the OS yet.
int64_t capacity =
(isolate_group->heap()->new_space()->CapacityInWords() +
(isolate_group->heap()->new_space()->UsedInWords() +
isolate_group->heap()->old_space()->CapacityInWords()) *
kWordSize;
// The more precise UsedInWords for new-space iterates pages and
// potentially accesses Thread::top_/end_, which is not thread-safe
// here. CapacityInWords is similar enough for purposes of service stats
// for new-space, differing only up to the as-yet-unused portion of
// active TLABs, unlike old-space where it can differ greatly in a
// highly fragmented heap.
int64_t used = (isolate_group->heap()->new_space()->CapacityInWords() +
isolate_group->heap()->old_space()->UsedInWords()) *
kWordSize;
int64_t used = isolate_group->heap()->TotalUsedInWords() * kWordSize;
int64_t free = capacity - used;
JSONObject group(&vm_children);