Skip to content

Commit a9dd011

Browse files
rmacnak-googleCommit Queue
authored andcommitted
[vm] Make out-of-memory scenarios a bit more robust.
- Don't scavenge when there is no OOM reservation, so refilling the OOM reservation will get the first go once free space is available instead it being consumed by promotions. - Increase the desired free space in new space needed to continue scavenging. Since new space became paged, 1 KB / 64 pages only guarentees the ability to allocate a 16 byte object. - Stop issuing stress test reloads after an OOM is encountered. - Add a HANDLESCOPE to reload to make memory collectible sooner after an error. TEST=vm/out_of_memory Bug: #48920 Bug: #53455 Bug: #43642 Change-Id: Ife8bed52e341648a66f78d55bfa60b3944be5d46 Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/422324 Commit-Queue: Ryan Macnak <[email protected]> Reviewed-by: Alexander Aprelev <[email protected]>
1 parent ef85087 commit a9dd011

File tree

6 files changed

+21
-5
lines changed

6 files changed

+21
-5
lines changed

runtime/vm/heap/heap.cc

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,8 @@ uword Heap::AllocateNew(Thread* thread, intptr_t size) {
8383
if (LIKELY(addr != 0)) {
8484
return addr;
8585
}
86-
if (!assume_scavenge_will_fail_ && !thread->force_growth()) {
86+
if (!assume_scavenge_will_fail_ && !thread->force_growth() &&
87+
old_space_.HasReservation()) {
8788
GcSafepointOperationScope safepoint_operation(thread);
8889

8990
// Another thread may have won the race to the safepoint and performed a GC
@@ -174,6 +175,7 @@ uword Heap::AllocateOld(Thread* thread, intptr_t size, bool is_exec) {
174175
// Give up allocating this object.
175176
OS::PrintErr("Exhausted heap space, trying to allocate %" Pd " bytes.\n",
176177
size);
178+
isolate_group_->set_has_seen_oom(true);
177179
return 0;
178180
}
179181

runtime/vm/heap/pages.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ class PageSpace {
168168
return AllocateSnapshotLockedSlow(freelist, size);
169169
}
170170

171+
bool HasReservation() { return oom_reservation_ != nullptr; }
171172
void TryReleaseReservation();
172173
bool MarkReservation();
173174
void TryReserveForOOM();
@@ -449,7 +450,10 @@ class PageSpace {
449450
kDataFreelist = 1,
450451
};
451452
FreeList* freelists_;
453+
452454
static constexpr intptr_t kOOMReservationSize = 32 * KB;
455+
static_assert(kOOMReservationSize < kAllocatablePageSize,
456+
"OOM reservation should not go to a large page");
453457
FreeListElement* oom_reservation_ = nullptr;
454458

455459
// Use ExclusivePageIterator for safe access to these.

runtime/vm/heap/scavenger.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1932,7 +1932,7 @@ void Scavenger::Scavenge(Thread* thread, GCType type, GCReason reason) {
19321932
ReverseScavenge(&from);
19331933
bytes_promoted = 0;
19341934
} else {
1935-
if ((ThresholdInWords() - UsedInWords()) < KBInWords) {
1935+
if ((ThresholdInWords() - UsedInWords()) < 32 * KBInWords) {
19361936
// Don't scavenge again until the next old-space GC has occurred. Prevents
19371937
// performing one scavenge per allocation as the heap limit is approached.
19381938
heap_->assume_scavenge_will_fail_ = true;

runtime/vm/isolate.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -481,6 +481,13 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
481481
#endif // !defined(DART_PRECOMPILED_RUNTIME)
482482
#endif // !defined(PRODUCT)
483483

484+
bool has_seen_oom() const {
485+
return HasSeenOOMBit::decode(isolate_group_flags_);
486+
}
487+
void set_has_seen_oom(bool value) {
488+
isolate_group_flags_ = HasSeenOOMBit::update(value, isolate_group_flags_);
489+
}
490+
484491
#if defined(PRODUCT)
485492
void set_use_osr(bool use_osr) { ASSERT(!use_osr); }
486493
#else // defined(PRODUCT)
@@ -801,6 +808,7 @@ class IsolateGroup : public IntrusiveDListEntry<IsolateGroup> {
801808
V(AllClassesFinalized) \
802809
V(EnableAsserts) \
803810
V(HasAttemptedReload) \
811+
V(HasSeenOOM) \
804812
V(RemappingCids) \
805813
V(ShouldLoadVmService) \
806814
V(Obfuscate) \

runtime/vm/isolate_reload.cc

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1220,7 +1220,8 @@ ObjectPtr ProgramReloadContext::ReloadPhase2LoadKernel(
12201220
const String& root_lib_url) {
12211221
Thread* thread = Thread::Current();
12221222

1223-
LongJumpScope jump;
1223+
HANDLESCOPE(thread);
1224+
LongJumpScope jump(thread);
12241225
if (DART_SETJMP(*jump.Set()) == 0) {
12251226
const Object& tmp = kernel::KernelLoader::LoadEntireProgram(program);
12261227
if (tmp.IsError()) {

runtime/vm/runtime_entry.cc

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3216,7 +3216,8 @@ static void HandleStackOverflowTestCases(Thread* thread) {
32163216
do_gc = true;
32173217
}
32183218
if ((isolate_reload_every > 0) && (count % isolate_reload_every) == 0) {
3219-
do_reload = isolate->group()->CanReload();
3219+
do_reload =
3220+
isolate_group->CanReload() && !isolate_group->has_seen_oom();
32203221
}
32213222
}
32223223
}
@@ -3275,7 +3276,7 @@ static void HandleStackOverflowTestCases(Thread* thread) {
32753276
JSONStream js;
32763277
const bool success =
32773278
isolate_group->ReloadSources(&js, /*force_reload=*/true, script_uri);
3278-
if (!success && !Dart::IsShuttingDown()) {
3279+
if (!success && !Dart::IsShuttingDown() && !isolate_group->has_seen_oom()) {
32793280
FATAL("*** Isolate reload failed:\n%s\n", js.ToCString());
32803281
}
32813282
}

0 commit comments

Comments
 (0)