From 5bdffcd32e6cd7c8aba3faaf0e38df7ce702c6fd Mon Sep 17 00:00:00 2001 From: Ryan Macnak Date: Tue, 17 Oct 2017 20:44:02 +0000 Subject: [PATCH] [vm] Add a simple compactor. The compactor copies all live objects in old space to fresh pages, places forwarding pointers in the old objects, forwards all the pointers, then frees the old pages. This has a high space overhead. It is not meant for use in production, but meant to test that the VM is properly set up to handle old-space objects moving. Large page objects and instruction objects are not moved. Bug: https://github.com/dart-lang/sdk/issues/30978 Change-Id: Ia42683fd5e27a33702aa5e83bece803a8b005a4b Reviewed-on: https://dart-review.googlesource.com/13624 Commit-Queue: Ryan Macnak Reviewed-by: Zach Anderson Reviewed-by: Erik Corry --- .../service/dominator_tree_user_test.dart | 1 + .../tests/service/dominator_tree_vm_test.dart | 1 + runtime/vm/become.cc | 85 +++++++------- runtime/vm/become.h | 5 + runtime/vm/flag_list.h | 2 + runtime/vm/gc_compactor.cc | 39 +++++++ runtime/vm/gc_compactor.h | 32 ++++++ runtime/vm/heap.cc | 5 + runtime/vm/isolate.h | 10 +- runtime/vm/isolate_reload.cc | 16 +-- runtime/vm/pages.cc | 108 ++++++++++++++++++ runtime/vm/pages.h | 2 + runtime/vm/profiler.cc | 17 ++- runtime/vm/raw_object.cc | 6 +- runtime/vm/stack_frame.cc | 18 ++- runtime/vm/vm_sources.gni | 2 + tests/lib/mirrors/mirrors_reader_test.dart | 3 + 17 files changed, 287 insertions(+), 65 deletions(-) create mode 100644 runtime/vm/gc_compactor.cc create mode 100644 runtime/vm/gc_compactor.h diff --git a/runtime/observatory/tests/service/dominator_tree_user_test.dart b/runtime/observatory/tests/service/dominator_tree_user_test.dart index e81f4705868..14572bcb3c0 100644 --- a/runtime/observatory/tests/service/dominator_tree_user_test.dart +++ b/runtime/observatory/tests/service/dominator_tree_user_test.dart @@ -2,6 +2,7 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. // VMOptions=--error_on_bad_type --error_on_bad_override +// VMOptions=--use_compactor import 'package:observatory/heap_snapshot.dart'; import 'package:observatory/models.dart' as M; diff --git a/runtime/observatory/tests/service/dominator_tree_vm_test.dart b/runtime/observatory/tests/service/dominator_tree_vm_test.dart index b78f82a676e..c04759c3715 100644 --- a/runtime/observatory/tests/service/dominator_tree_vm_test.dart +++ b/runtime/observatory/tests/service/dominator_tree_vm_test.dart @@ -2,6 +2,7 @@ // for details. All rights reserved. Use of this source code is governed by a // BSD-style license that can be found in the LICENSE file. // VMOptions=--error_on_bad_type --error_on_bad_override +// VMOptions=--use_compactor import 'package:observatory/heap_snapshot.dart'; import 'package:observatory/models.dart' as M; diff --git a/runtime/vm/become.cc b/runtime/vm/become.cc index b34a338c474..7d53171ab38 100644 --- a/runtime/vm/become.cc +++ b/runtime/vm/become.cc @@ -76,8 +76,7 @@ class ForwardPointersVisitor : public ObjectPointerVisitor { explicit ForwardPointersVisitor(Thread* thread) : ObjectPointerVisitor(thread->isolate()), thread_(thread), - visiting_object_(NULL), - count_(0) {} + visiting_object_(NULL) {} virtual void VisitPointers(RawObject** first, RawObject** last) { for (RawObject** p = first; p <= last; p++) { @@ -89,7 +88,6 @@ class ForwardPointersVisitor : public ObjectPointerVisitor { } else { visiting_object_->StorePointer(p, new_target); } - count_++; } } } @@ -103,12 +101,9 @@ class ForwardPointersVisitor : public ObjectPointerVisitor { } } - intptr_t count() const { return count_; } - private: Thread* thread_; RawObject* visiting_object_; - intptr_t count_; DISALLOW_COPY_AND_ASSIGN(ForwardPointersVisitor); }; @@ -131,23 +126,18 @@ class ForwardHeapPointersVisitor : public ObjectVisitor { class ForwardHeapPointersHandleVisitor : public HandleVisitor { public: - ForwardHeapPointersHandleVisitor() - : HandleVisitor(Thread::Current()), count_(0) {} + explicit ForwardHeapPointersHandleVisitor(Thread* thread) + : HandleVisitor(thread) {} virtual void VisitHandle(uword addr) { FinalizablePersistentHandle* handle = reinterpret_cast(addr); if (IsForwardingObject(handle->raw())) { *handle->raw_addr() = GetForwardedObject(handle->raw()); - count_++; } } - intptr_t count() const { return count_; } - private: - int count_; - DISALLOW_COPY_AND_ASSIGN(ForwardHeapPointersHandleVisitor); }; @@ -226,7 +216,7 @@ void Become::ElementsForwardIdentity(const Array& before, const Array& after) { Heap* heap = isolate->heap(); TIMELINE_FUNCTION_GC_DURATION(thread, "Become::ElementsForwardIdentity"); - HeapIterationScope his(thread); + SafepointOperationScope safepoint(thread); // Setup forwarding pointers. ASSERT(before.Length() == after.Length()); @@ -261,42 +251,12 @@ void Become::ElementsForwardIdentity(const Array& before, const Array& after) { ForwardObjectTo(before_obj, after_obj); heap->ForwardWeakEntries(before_obj, after_obj); - #if defined(HASH_IN_OBJECT_HEADER) Object::SetCachedHash(after_obj, Object::GetCachedHash(before_obj)); #endif } - { - // Follow forwarding pointers. - - // Clear the store buffer; will be rebuilt as we forward the heap. - isolate->PrepareForGC(); // Have all threads flush their store buffers. - isolate->store_buffer()->Reset(); // Drop all store buffers. - - // C++ pointers - ForwardPointersVisitor pointer_visitor(thread); - isolate->VisitObjectPointers(&pointer_visitor, true); - - // Weak persistent handles. - ForwardHeapPointersHandleVisitor handle_visitor; - isolate->VisitWeakPersistentHandles(&handle_visitor); - - // Heap pointers (may require updating the remembered set) - { - WritableCodeLiteralsScope writable_code(heap); - ForwardHeapPointersVisitor object_visitor(&pointer_visitor); - heap->VisitObjects(&object_visitor); - pointer_visitor.VisitingObject(NULL); - } - -#if !defined(PRODUCT) - tds.SetNumArguments(2); - tds.FormatArgument(0, "Remapped objects", "%" Pd, before.Length()); - tds.FormatArgument(1, "Remapped references", "%" Pd, - pointer_visitor.count() + handle_visitor.count()); -#endif - } + FollowForwardingPointers(thread); #if defined(DEBUG) for (intptr_t i = 0; i < before.Length(); i++) { @@ -305,4 +265,39 @@ void Become::ElementsForwardIdentity(const Array& before, const Array& after) { #endif } +void Become::FollowForwardingPointers(Thread* thread) { + // N.B.: We forward the heap before forwarding the stack. This limits the + // amount of following of forwarding pointers needed to get at stack maps. + Isolate* isolate = thread->isolate(); + Heap* heap = isolate->heap(); + + // Clear the store buffer; will be rebuilt as we forward the heap. + isolate->PrepareForGC(); // Have all threads flush their store buffers. + isolate->store_buffer()->Reset(); // Drop all store buffers. + + ForwardPointersVisitor pointer_visitor(thread); + + { + // Heap pointers. + WritableCodeLiteralsScope writable_code(heap); + ForwardHeapPointersVisitor object_visitor(&pointer_visitor); + heap->VisitObjects(&object_visitor); + pointer_visitor.VisitingObject(NULL); + } + + // C++ pointers. + isolate->VisitObjectPointers(&pointer_visitor, true); +#ifndef PRODUCT + if (FLAG_support_service) { + ObjectIdRing* ring = isolate->object_id_ring(); + ASSERT(ring != NULL); + ring->VisitPointers(&pointer_visitor); + } +#endif // !PRODUCT + + // Weak persistent handles. + ForwardHeapPointersHandleVisitor handle_visitor(thread); + isolate->VisitWeakPersistentHandles(&handle_visitor); +} + } // namespace dart diff --git a/runtime/vm/become.h b/runtime/vm/become.h index aaa60e18b9d..ca25361ed53 100644 --- a/runtime/vm/become.h +++ b/runtime/vm/become.h @@ -80,6 +80,11 @@ class Become : public AllStatic { // Useful for atomically applying behavior and schema changes. static void ElementsForwardIdentity(const Array& before, const Array& after); + // Update any references pointing to forwarding objects to point the + // forwarding objects' targets. Used by the implementation of become and the + // simplistic compactor. + static void FollowForwardingPointers(Thread* thread); + // Convert and instance object into a dummy object, // making the instance independent of its class. // (used for morphic instances during reload). diff --git a/runtime/vm/flag_list.h b/runtime/vm/flag_list.h index e83e818f112..36346dfaabe 100644 --- a/runtime/vm/flag_list.h +++ b/runtime/vm/flag_list.h @@ -170,6 +170,8 @@ D(trace_zones, bool, false, "Traces allocation sizes in the zone.") \ P(truncating_left_shift, bool, true, \ "Optimize left shift to truncate if possible") \ + R(use_compactor, false, bool, false, \ + "Compact the heap during old-space GC.") \ P(use_cha_deopt, bool, true, \ "Use class hierarchy analysis even if it can cause deoptimization.") \ P(use_field_guards, bool, !USING_DBC, \ diff --git a/runtime/vm/gc_compactor.cc b/runtime/vm/gc_compactor.cc new file mode 100644 index 00000000000..52ecac99628 --- /dev/null +++ b/runtime/vm/gc_compactor.cc @@ -0,0 +1,39 @@ +// Copyright (c) 2017, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +#include "vm/gc_compactor.h" + +#include "vm/become.h" +#include "vm/globals.h" +#include "vm/heap.h" +#include "vm/pages.h" + +namespace dart { + +void GCCompactor::EvacuatePage(HeapPage* page) { + uword current = page->object_start(); + uword end = page->object_end(); + while (current < end) { + RawObject* raw_obj = RawObject::FromAddr(current); + const intptr_t size = raw_obj->Size(); + if (!raw_obj->IsFreeListElement() && !raw_obj->IsForwardingCorpse()) { + uword new_obj = heap_->old_space()->TryAllocateDataLocked( + size, PageSpace::kForceGrowth); + if (new_obj == 0) { + OUT_OF_MEMORY(); + } + + memmove(reinterpret_cast(new_obj), + reinterpret_cast(current), size); + + ForwardingCorpse* forwarder = + ForwardingCorpse::AsForwarder(current, size); + forwarder->set_target(RawObject::FromAddr(new_obj)); + heap_->ForwardWeakEntries(raw_obj, RawObject::FromAddr(new_obj)); + } + current += size; + } +} + +} // namespace dart diff --git a/runtime/vm/gc_compactor.h b/runtime/vm/gc_compactor.h new file mode 100644 index 00000000000..ff5b057265a --- /dev/null +++ b/runtime/vm/gc_compactor.h @@ -0,0 +1,32 @@ +// Copyright (c) 2017, the Dart project authors. Please see the AUTHORS file +// for details. All rights reserved. Use of this source code is governed by a +// BSD-style license that can be found in the LICENSE file. + +#ifndef RUNTIME_VM_GC_COMPACTOR_H_ +#define RUNTIME_VM_GC_COMPACTOR_H_ + +#include "vm/allocation.h" +#include "vm/globals.h" + +namespace dart { + +// Forward declarations. +class HeapPage; +class Heap; + +// The class GCCompactor is used to relocate objects to fresh pages to remove +// fragmentation. +class GCCompactor : public ValueObject { + public: + explicit GCCompactor(Heap* heap) : heap_(heap) {} + ~GCCompactor() {} + + void EvacuatePage(HeapPage* page); + + private: + Heap* heap_; +}; + +} // namespace dart + +#endif // RUNTIME_VM_GC_COMPACTOR_H_ diff --git a/runtime/vm/heap.cc b/runtime/vm/heap.cc index c90dc3a7f51..b1c7d965a04 100644 --- a/runtime/vm/heap.cc +++ b/runtime/vm/heap.cc @@ -422,6 +422,11 @@ void Heap::CollectOldSpaceGarbage(Thread* thread, TIMELINE_FUNCTION_GC_DURATION_BASIC(thread, "CollectOldGeneration"); NOT_IN_PRODUCT(UpdateClassHeapStatsBeforeGC(kOld)); old_space_.MarkSweep(); +#ifndef PRODUCT + if (FLAG_use_compactor) { + old_space_.Compact(); + } +#endif RecordAfterGC(kOld); PrintStats(); NOT_IN_PRODUCT(PrintStatsToTimeline(&tds)); diff --git a/runtime/vm/isolate.h b/runtime/vm/isolate.h index 09a81091bd0..50e904317e7 100644 --- a/runtime/vm/isolate.h +++ b/runtime/vm/isolate.h @@ -305,6 +305,13 @@ class Isolate : public BaseIsolate { #endif } + bool compaction_in_progress() const { + return CompactionInProgressBit::decode(isolate_flags_); + } + void set_compaction_in_progress(bool value) { + isolate_flags_ = CompactionInProgressBit::update(value, isolate_flags_); + } + IsolateSpawnState* spawn_state() const { return spawn_state_; } void set_spawn_state(IsolateSpawnState* value) { spawn_state_ = value; } @@ -835,7 +842,8 @@ class Isolate : public BaseIsolate { V(ErrorOnBadOverride) \ V(UseFieldGuards) \ V(UseOsr) \ - V(Obfuscate) + V(Obfuscate) \ + V(CompactionInProgress) // Isolate specific flags. enum FlagBits { diff --git a/runtime/vm/isolate_reload.cc b/runtime/vm/isolate_reload.cc index 830d30d0b7a..28c74f16af2 100644 --- a/runtime/vm/isolate_reload.cc +++ b/runtime/vm/isolate_reload.cc @@ -1380,13 +1380,15 @@ void IsolateReloadContext::Commit() { RehashConstants(); #ifdef DEBUG - // Verify that all canonical instances are correctly setup in the - // corresponding canonical tables. - Thread* thread = Thread::Current(); - I->heap()->CollectAllGarbage(); - HeapIterationScope iteration(thread); - VerifyCanonicalVisitor check_canonical(thread); - iteration.IterateObjects(&check_canonical); + { + // Verify that all canonical instances are correctly setup in the + // corresponding canonical tables. + Thread* thread = Thread::Current(); + I->heap()->CollectAllGarbage(); + HeapIterationScope iteration(thread); + VerifyCanonicalVisitor check_canonical(thread); + iteration.IterateObjects(&check_canonical); + } #endif // DEBUG if (FLAG_identity_reload) { diff --git a/runtime/vm/pages.cc b/runtime/vm/pages.cc index fd8545f3273..fe86ef47251 100644 --- a/runtime/vm/pages.cc +++ b/runtime/vm/pages.cc @@ -6,7 +6,9 @@ #include "platform/address_sanitizer.h" #include "platform/assert.h" +#include "vm/become.h" #include "vm/compiler_stats.h" +#include "vm/gc_compactor.h" #include "vm/gc_marker.h" #include "vm/gc_sweeper.h" #include "vm/lockers.h" @@ -1044,6 +1046,112 @@ void PageSpace::MarkSweep() { } } +void PageSpace::Compact() { + for (HeapPage* page = pages_; page != NULL; page = page->next()) { + if (page->is_image_page()) { + // Implementation doesn't currently handle image pages. + return; + } + } + + Thread* thread = Thread::Current(); + + // Wait for sweeper tasks. + { + MonitorLocker ml(tasks_lock()); + while (tasks() > 0) { + ml.WaitWithSafepointCheck(thread); + } + set_tasks(1); + } + + { + SafepointOperationScope safepoint(thread); + thread->isolate()->set_compaction_in_progress(true); + + // Note this excludes code pages and large objects pages. + HeapPage* pages_to_evacuate = pages_; + + // Prevent allocation into these pages during evacuation. + pages_ = pages_tail_ = NULL; + AbandonBumpAllocation(); + freelist_[HeapPage::kData].Reset(); + + // Evacuate. + { + AcquireDataLock(); + + GCCompactor compactor(heap_); + HeapPage* page = pages_to_evacuate; + while (page != NULL) { + if (page->is_image_page()) { + UNIMPLEMENTED(); + } + ASSERT(page->type() != HeapPage::kExecutable); + compactor.EvacuatePage(page); + page = page->next(); + } + + ReleaseDataLock(); + } + + // Forward. + Become::FollowForwardingPointers(thread); + + // Free. + HeapPage* page = pages_to_evacuate; + while (page != NULL) { + if (page->is_image_page()) { + UNIMPLEMENTED(); + } + ASSERT(page->type() != HeapPage::kExecutable); + HeapPage* next = page->next(); + page->Deallocate(); + page = next; + } + + if (FLAG_verify_after_gc) { + OS::PrintErr("Verifying after compacting..."); + heap_->VerifyGC(kForbidMarked); + OS::PrintErr(" done.\n"); + } + + thread->isolate()->set_compaction_in_progress(false); + } + + // Done, reset the task count. + { + MonitorLocker ml(tasks_lock()); + set_tasks(tasks() - 1); + ml.NotifyAll(); + } + + { + // Const object tables are hashed by address: rehash. + SafepointOperationScope safepoint(thread); + StackZone zone(thread); + Isolate* I = thread->isolate(); + + ClassTable* class_table = I->class_table(); + Class& cls = Class::Handle(zone.GetZone()); + const intptr_t top = class_table->NumCids(); + for (intptr_t cid = kInstanceCid; cid < top; cid++) { + if (!class_table->IsValidIndex(cid) || + !class_table->HasValidClassAt(cid)) { + continue; + } + if ((cid == kTypeArgumentsCid) || RawObject::IsStringClassId(cid)) { + // TypeArguments and Symbols have special tables for canonical objects + // that aren't based on address. + continue; + } + // Rehash constants. + cls = class_table->At(cid); + cls.RehashConstants(zone.GetZone()); // May allocate. + } + } +} + uword PageSpace::TryAllocateDataBumpInternal(intptr_t size, GrowthPolicy growth_policy, bool is_locked) { diff --git a/runtime/vm/pages.h b/runtime/vm/pages.h index 859fce32efd..17a5a70fae8 100644 --- a/runtime/vm/pages.h +++ b/runtime/vm/pages.h @@ -257,6 +257,8 @@ class PageSpace { // Collect the garbage in the page space using mark-sweep. void MarkSweep(); + // Compact the heap using evacuation. + void Compact(); void AddRegionsToObjectSet(ObjectSet* set) const; diff --git a/runtime/vm/profiler.cc b/runtime/vm/profiler.cc index f22ba8030de..ebcebc1e84b 100644 --- a/runtime/vm/profiler.cc +++ b/runtime/vm/profiler.cc @@ -1333,11 +1333,18 @@ void Profiler::SampleThread(Thread* thread, return; } - if (thread->IsMutatorThread() && isolate->IsDeoptimizing()) { - AtomicOperations::IncrementInt64By( - &counters_.single_frame_sample_deoptimizing, 1); - SampleThreadSingleFrame(thread, pc); - return; + if (thread->IsMutatorThread()) { + if (isolate->IsDeoptimizing()) { + AtomicOperations::IncrementInt64By( + &counters_.single_frame_sample_deoptimizing, 1); + SampleThreadSingleFrame(thread, pc); + return; + } + if (isolate->compaction_in_progress()) { + // The Dart stack isn't fully walkable. + SampleThreadSingleFrame(thread, pc); + return; + } } if (!InitialRegisterCheck(pc, fp, sp)) { diff --git a/runtime/vm/raw_object.cc b/runtime/vm/raw_object.cc index 05a9735353f..598b8349166 100644 --- a/runtime/vm/raw_object.cc +++ b/runtime/vm/raw_object.cc @@ -543,11 +543,13 @@ intptr_t RawObjectPool::VisitObjectPoolPointers(RawObjectPool* raw_obj, visitor->VisitPointer( reinterpret_cast(&raw_obj->ptr()->info_array_)); const intptr_t len = raw_obj->ptr()->length_; - RawTypedData* info_array = raw_obj->ptr()->info_array_->ptr(); + RawTypedData* info_array = raw_obj->ptr()->info_array_; + ASSERT(!info_array->IsForwardingCorpse()); + Entry* first = raw_obj->first_entry(); for (intptr_t i = 0; i < len; ++i) { ObjectPool::EntryType entry_type = - static_cast(info_array->data()[i]); + static_cast(info_array->ptr()->data()[i]); if (entry_type == ObjectPool::kTaggedObject) { visitor->VisitPointer(&(first + i)->raw_obj_); } diff --git a/runtime/vm/stack_frame.cc b/runtime/vm/stack_frame.cc index 9537968b186..052f34446ab 100644 --- a/runtime/vm/stack_frame.cc +++ b/runtime/vm/stack_frame.cc @@ -5,6 +5,7 @@ #include "vm/stack_frame.h" #include "platform/memory_sanitizer.h" +#include "vm/become.h" #include "vm/compiler/assembler/assembler.h" #include "vm/deopt_instructions.h" #include "vm/isolate.h" @@ -238,12 +239,19 @@ RawCode* StackFrame::LookupDartCode() const { } RawCode* StackFrame::GetCodeObject() const { - const uword pc_marker = - *(reinterpret_cast(fp() + (kPcMarkerSlotFromFp * kWordSize))); + RawObject* pc_marker = *( + reinterpret_cast(fp() + (kPcMarkerSlotFromFp * kWordSize))); ASSERT(pc_marker != 0); - ASSERT(reinterpret_cast(pc_marker)->GetClassId() == kCodeCid || - reinterpret_cast(pc_marker) == Object::null()); - return reinterpret_cast(pc_marker); + // When forwarding the stack, we look at the pc marker to get the frame's + // stack map before we've forwarded the pc marker. + if (pc_marker->GetClassId() == kForwardingCorpse) { + const uword addr = reinterpret_cast(pc_marker) - kHeapObjectTag; + ForwardingCorpse* forwarder = reinterpret_cast(addr); + pc_marker = forwarder->target(); + } + ASSERT((pc_marker == Object::null()) || + (pc_marker->GetClassId() == kCodeCid)); + return static_cast(pc_marker); } bool StackFrame::FindExceptionHandler(Thread* thread, diff --git a/runtime/vm/vm_sources.gni b/runtime/vm/vm_sources.gni index 22c2bdf5b89..8231b65caf2 100644 --- a/runtime/vm/vm_sources.gni +++ b/runtime/vm/vm_sources.gni @@ -104,6 +104,8 @@ vm_sources = [ "flags.h", "freelist.cc", "freelist.h", + "gc_compactor.cc", + "gc_compactor.h", "gc_marker.cc", "gc_marker.h", "gc_sweeper.cc", diff --git a/tests/lib/mirrors/mirrors_reader_test.dart b/tests/lib/mirrors/mirrors_reader_test.dart index 8717f45f551..4cc9497a93e 100644 --- a/tests/lib/mirrors/mirrors_reader_test.dart +++ b/tests/lib/mirrors/mirrors_reader_test.dart @@ -4,6 +4,9 @@ // Test that everything reachable from a [MirrorSystem] can be accessed. +// VMOptions= +// VMOptions=--use_compactor + library test.mirrors.reader; import 'dart:mirrors';