From ff61743fb0c6735d37fc9d436081928edea68aae Mon Sep 17 00:00:00 2001 From: Ulan Degenbaev Date: Thu, 1 Oct 2020 14:07:28 +0200 Subject: [PATCH] [heap] Refactor marking weak object worklists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This CL extracts weak object worklist related code into separate files and uses a macro to specify all weak object worklists in a generic way. The motivation of the refactoring is twofold: 1) We can now enforce that each weak object worklist is updated after Scavenge. (Forgetting to define the update function causes a link time error.) 2) The reduced boilerplate will be useful for transitioning to the new ::heap::base::Worklist. Change-Id: Ic80a7ccca010c09370d6525f43d78de24192f8ea Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/2442624 Reviewed-by: Dominik Inführ Commit-Queue: Ulan Degenbaev Cr-Commit-Position: refs/heads/master@{#70308} --- BUILD.gn | 2 + src/heap/concurrent-marking.h | 2 +- src/heap/incremental-marking.cc | 104 +----------------- src/heap/incremental-marking.h | 1 - src/heap/marking-visitor.h | 48 +-------- src/heap/weak-object-worklists.cc | 172 ++++++++++++++++++++++++++++++ src/heap/weak-object-worklists.h | 90 ++++++++++++++++ 7 files changed, 267 insertions(+), 152 deletions(-) create mode 100644 src/heap/weak-object-worklists.cc create mode 100644 src/heap/weak-object-worklists.h diff --git a/BUILD.gn b/BUILD.gn index bd7e8c82df..8369cd9c37 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -2678,6 +2678,8 @@ v8_source_set("v8_base_without_compiler") { "src/heap/stress-scavenge-observer.h", "src/heap/sweeper.cc", "src/heap/sweeper.h", + "src/heap/weak-object-worklists.cc", + "src/heap/weak-object-worklists.h", "src/heap/worklist.h", "src/ic/call-optimization.cc", "src/ic/call-optimization.h", diff --git a/src/heap/concurrent-marking.h b/src/heap/concurrent-marking.h index 6ed671fb1b..f2a3f827c3 100644 --- a/src/heap/concurrent-marking.h +++ b/src/heap/concurrent-marking.h @@ -29,7 +29,7 @@ class Heap; class Isolate; class MajorNonAtomicMarkingState; class MemoryChunk; -struct WeakObjects; +class WeakObjects; struct MemoryChunkData { intptr_t live_bytes; diff --git a/src/heap/incremental-marking.cc b/src/heap/incremental-marking.cc index c5206adf81..57f082dc77 100644 --- a/src/heap/incremental-marking.cc +++ b/src/heap/incremental-marking.cc @@ -501,109 +501,7 @@ void IncrementalMarking::UpdateMarkingWorklistAfterScavenge() { } }); - UpdateWeakReferencesAfterScavenge(); -} - -void IncrementalMarking::UpdateWeakReferencesAfterScavenge() { - weak_objects_->weak_references.Update( - [](std::pair slot_in, - std::pair* slot_out) -> bool { - HeapObject heap_obj = slot_in.first; - HeapObject forwarded = ForwardingAddress(heap_obj); - - if (!forwarded.is_null()) { - ptrdiff_t distance_to_slot = - slot_in.second.address() - slot_in.first.ptr(); - Address new_slot = forwarded.ptr() + distance_to_slot; - slot_out->first = forwarded; - slot_out->second = HeapObjectSlot(new_slot); - return true; - } - - return false; - }); - weak_objects_->weak_objects_in_code.Update( - [](std::pair slot_in, - std::pair* slot_out) -> bool { - HeapObject heap_obj = slot_in.first; - HeapObject forwarded = ForwardingAddress(heap_obj); - - if (!forwarded.is_null()) { - slot_out->first = forwarded; - slot_out->second = slot_in.second; - return true; - } - - return false; - }); - weak_objects_->ephemeron_hash_tables.Update( - [](EphemeronHashTable slot_in, EphemeronHashTable* slot_out) -> bool { - EphemeronHashTable forwarded = ForwardingAddress(slot_in); - - if (!forwarded.is_null()) { - *slot_out = forwarded; - return true; - } - - return false; - }); - - auto ephemeron_updater = [](Ephemeron slot_in, Ephemeron* slot_out) -> bool { - HeapObject key = slot_in.key; - HeapObject value = slot_in.value; - HeapObject forwarded_key = ForwardingAddress(key); - HeapObject forwarded_value = ForwardingAddress(value); - - if (!forwarded_key.is_null() && !forwarded_value.is_null()) { - *slot_out = Ephemeron{forwarded_key, forwarded_value}; - return true; - } - - return false; - }; - - weak_objects_->current_ephemerons.Update(ephemeron_updater); - weak_objects_->next_ephemerons.Update(ephemeron_updater); - weak_objects_->discovered_ephemerons.Update(ephemeron_updater); - - weak_objects_->flushed_js_functions.Update( - [](JSFunction slot_in, JSFunction* slot_out) -> bool { - JSFunction forwarded = ForwardingAddress(slot_in); - - if (!forwarded.is_null()) { - *slot_out = forwarded; - return true; - } - - return false; - }); -#ifdef DEBUG - weak_objects_->bytecode_flushing_candidates.Iterate( - [](SharedFunctionInfo candidate) { - DCHECK(!Heap::InYoungGeneration(candidate)); - }); -#endif - - if (FLAG_harmony_weak_refs) { - weak_objects_->js_weak_refs.Update( - [](JSWeakRef js_weak_ref_in, JSWeakRef* js_weak_ref_out) -> bool { - JSWeakRef forwarded = ForwardingAddress(js_weak_ref_in); - - if (!forwarded.is_null()) { - *js_weak_ref_out = forwarded; - return true; - } - - return false; - }); - -#ifdef DEBUG - // TODO(syg, marja): Support WeakCells in the young generation. - weak_objects_->weak_cells.Iterate([](WeakCell weak_cell) { - DCHECK(!Heap::InYoungGeneration(weak_cell)); - }); -#endif - } + weak_objects_->UpdateAfterScavenge(); } void IncrementalMarking::UpdateMarkedBytesAfterScavenge( diff --git a/src/heap/incremental-marking.h b/src/heap/incremental-marking.h index 29df137711..4e3df832c3 100644 --- a/src/heap/incremental-marking.h +++ b/src/heap/incremental-marking.h @@ -146,7 +146,6 @@ class V8_EXPORT_PRIVATE IncrementalMarking final { void FinalizeIncrementally(); void UpdateMarkingWorklistAfterScavenge(); - void UpdateWeakReferencesAfterScavenge(); void UpdateMarkedBytesAfterScavenge(size_t dead_bytes_in_new_space); void Hurry(); diff --git a/src/heap/marking-visitor.h b/src/heap/marking-visitor.h index 37219a2385..45dda338d0 100644 --- a/src/heap/marking-visitor.h +++ b/src/heap/marking-visitor.h @@ -11,58 +11,12 @@ #include "src/heap/memory-chunk.h" #include "src/heap/objects-visiting.h" #include "src/heap/spaces.h" +#include "src/heap/weak-object-worklists.h" #include "src/heap/worklist.h" -#include "src/objects/heap-object.h" // For Worklist -#include "src/objects/js-weak-refs.h" // For Worklist namespace v8 { namespace internal { -struct Ephemeron { - HeapObject key; - HeapObject value; -}; - -using EphemeronWorklist = Worklist; - -// Weak objects encountered during marking. -struct WeakObjects { - Worklist transition_arrays; - - // Keep track of all EphemeronHashTables in the heap to process - // them in the atomic pause. - Worklist ephemeron_hash_tables; - - // Keep track of all ephemerons for concurrent marking tasks. Only store - // ephemerons in these Worklists if both key and value are unreachable at the - // moment. - // - // MarkCompactCollector::ProcessEphemeronsUntilFixpoint drains and fills these - // worklists. - // - // current_ephemerons is used as draining worklist in the current fixpoint - // iteration. - EphemeronWorklist current_ephemerons; - - // Stores ephemerons to visit in the next fixpoint iteration. - EphemeronWorklist next_ephemerons; - - // When draining the marking worklist new discovered ephemerons are pushed - // into this worklist. - EphemeronWorklist discovered_ephemerons; - - // TODO(marja): For old space, we only need the slot, not the host - // object. Optimize this by adding a different storage for old space. - Worklist, 64> weak_references; - Worklist, 64> weak_objects_in_code; - - Worklist js_weak_refs; - Worklist weak_cells; - - Worklist bytecode_flushing_candidates; - Worklist flushed_js_functions; -}; - struct EphemeronMarking { std::vector newly_discovered; bool newly_discovered_overflowed; diff --git a/src/heap/weak-object-worklists.cc b/src/heap/weak-object-worklists.cc new file mode 100644 index 0000000000..532739000f --- /dev/null +++ b/src/heap/weak-object-worklists.cc @@ -0,0 +1,172 @@ +// Copyright 2020 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "src/heap/weak-object-worklists.h" + +#include "src/heap/heap-inl.h" +#include "src/heap/heap.h" +#include "src/heap/worklist.h" +#include "src/objects/hash-table.h" +#include "src/objects/heap-object.h" +#include "src/objects/js-function.h" +#include "src/objects/js-weak-refs-inl.h" +#include "src/objects/js-weak-refs.h" +#include "src/objects/shared-function-info.h" +#include "src/objects/transitions.h" + +namespace v8 { + +namespace internal { + +void WeakObjects::UpdateAfterScavenge() { +#define INVOKE_UPDATE(_, name, Name) Update##Name(name); + WEAK_OBJECT_WORKLISTS(INVOKE_UPDATE) +#undef INVOKE_UPDATE +} + +void WeakObjects::UpdateTransitionArrays( + WeakObjectWorklist& transition_arrays) { + DCHECK(!ContainsYoungObjects(transition_arrays)); +} + +void WeakObjects::UpdateEphemeronHashTables( + WeakObjectWorklist& ephemeron_hash_tables) { + ephemeron_hash_tables.Update( + [](EphemeronHashTable slot_in, EphemeronHashTable* slot_out) -> bool { + EphemeronHashTable forwarded = ForwardingAddress(slot_in); + + if (!forwarded.is_null()) { + *slot_out = forwarded; + return true; + } + + return false; + }); +} + +namespace { +bool EphemeronUpdater(Ephemeron slot_in, Ephemeron* slot_out) { + HeapObject key = slot_in.key; + HeapObject value = slot_in.value; + HeapObject forwarded_key = ForwardingAddress(key); + HeapObject forwarded_value = ForwardingAddress(value); + + if (!forwarded_key.is_null() && !forwarded_value.is_null()) { + *slot_out = Ephemeron{forwarded_key, forwarded_value}; + return true; + } + + return false; +} +} // anonymous namespace + +void WeakObjects::UpdateCurrentEphemerons( + WeakObjectWorklist& current_ephemerons) { + current_ephemerons.Update(EphemeronUpdater); +} + +void WeakObjects::UpdateNextEphemerons( + WeakObjectWorklist& next_ephemerons) { + next_ephemerons.Update(EphemeronUpdater); +} + +void WeakObjects::UpdateDiscoveredEphemerons( + WeakObjectWorklist& discovered_ephemerons) { + discovered_ephemerons.Update(EphemeronUpdater); +} + +void WeakObjects::UpdateWeakReferences( + WeakObjectWorklist& weak_references) { + weak_references.Update( + [](HeapObjectAndSlot slot_in, HeapObjectAndSlot* slot_out) -> bool { + HeapObject heap_obj = slot_in.first; + HeapObject forwarded = ForwardingAddress(heap_obj); + + if (!forwarded.is_null()) { + ptrdiff_t distance_to_slot = + slot_in.second.address() - slot_in.first.ptr(); + Address new_slot = forwarded.ptr() + distance_to_slot; + slot_out->first = forwarded; + slot_out->second = HeapObjectSlot(new_slot); + return true; + } + + return false; + }); +} + +void WeakObjects::UpdateWeakObjectsInCode( + WeakObjectWorklist& weak_objects_in_code) { + weak_objects_in_code.Update( + [](HeapObjectAndCode slot_in, HeapObjectAndCode* slot_out) -> bool { + HeapObject heap_obj = slot_in.first; + HeapObject forwarded = ForwardingAddress(heap_obj); + + if (!forwarded.is_null()) { + slot_out->first = forwarded; + slot_out->second = slot_in.second; + return true; + } + + return false; + }); +} + +void WeakObjects::UpdateJSWeakRefs( + WeakObjectWorklist& js_weak_refs) { + if (FLAG_harmony_weak_refs) { + js_weak_refs.Update( + [](JSWeakRef js_weak_ref_in, JSWeakRef* js_weak_ref_out) -> bool { + JSWeakRef forwarded = ForwardingAddress(js_weak_ref_in); + + if (!forwarded.is_null()) { + *js_weak_ref_out = forwarded; + return true; + } + + return false; + }); + } +} + +void WeakObjects::UpdateWeakCells(WeakObjectWorklist& weak_cells) { + // TODO(syg, marja): Support WeakCells in the young generation. + DCHECK(!ContainsYoungObjects(weak_cells)); +} + +void WeakObjects::UpdateBytecodeFlushingCandidates( + WeakObjectWorklist& bytecode_flushing_candidates) { + DCHECK(!ContainsYoungObjects(bytecode_flushing_candidates)); +} + +void WeakObjects::UpdateFlushedJSFunctions( + WeakObjectWorklist& flushed_js_functions) { + flushed_js_functions.Update( + [](JSFunction slot_in, JSFunction* slot_out) -> bool { + JSFunction forwarded = ForwardingAddress(slot_in); + + if (!forwarded.is_null()) { + *slot_out = forwarded; + return true; + } + + return false; + }); +} + +#ifdef DEBUG +template +bool WeakObjects::ContainsYoungObjects(WeakObjectWorklist& worklist) { + bool result = false; + worklist.Iterate([&result](Type candidate) { + if (Heap::InYoungGeneration(candidate)) { + result = true; + } + }); + return result; +} +#endif + +} // namespace internal +} // namespace v8 diff --git a/src/heap/weak-object-worklists.h b/src/heap/weak-object-worklists.h new file mode 100644 index 0000000000..67df372b57 --- /dev/null +++ b/src/heap/weak-object-worklists.h @@ -0,0 +1,90 @@ +// Copyright 2020 the V8 project authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef V8_HEAP_WEAK_OBJECT_WORKLISTS_H_ +#define V8_HEAP_WEAK_OBJECT_WORKLISTS_H_ + +#include "src/common/globals.h" +#include "src/heap/worklist.h" +#include "src/objects/heap-object.h" +#include "src/objects/js-weak-refs.h" + +namespace v8 { +namespace internal { + +struct Ephemeron { + HeapObject key; + HeapObject value; +}; + +using HeapObjectAndSlot = std::pair; +using HeapObjectAndCode = std::pair; +class EphemeronHashTable; +class JSFunction; +class SharedFunctionInfo; +class TransitionArray; + +// Weak objects and weak references discovered during incremental/concurrent +// marking. They are processed in ClearNonLiveReferences after marking. +// Each entry in this list specifies: +// 1) Type of the worklist entry. +// 2) Lower-case name of the worklsit. +// 3) Capitalized name of the worklist. +// +// If you add a new entry, then you also need to implement the corresponding +// Update*() function in the cc file for updating pointers after Scavenge. +#define WEAK_OBJECT_WORKLISTS(F) \ + F(TransitionArray, transition_arrays, TransitionArrays) \ + /* Keep track of all EphemeronHashTables in the heap to process \ + them in the atomic pause. */ \ + F(EphemeronHashTable, ephemeron_hash_tables, EphemeronHashTables) \ + /* Keep track of all ephemerons for concurrent marking tasks. Only store \ + ephemerons in these worklists if both (key, value) are unreachable at \ + the moment. \ + MarkCompactCollector::ProcessEphemeronsUntilFixpoint drains/fills \ + these worklists. current_ephemerons is used as draining worklist in \ + the current fixpoint iteration. */ \ + F(Ephemeron, current_ephemerons, CurrentEphemerons) \ + /* Stores ephemerons to visit in the next fixpoint iteration. */ \ + F(Ephemeron, next_ephemerons, NextEphemerons) \ + /* When draining the marking worklist new discovered ephemerons are pushed \ + into this worklist. */ \ + F(Ephemeron, discovered_ephemerons, DiscoveredEphemerons) \ + /* TODO(marja): For old space, we only need the slot, not the host object. \ + Optimize this by adding a different storage for old space. */ \ + F(HeapObjectAndSlot, weak_references, WeakReferences) \ + F(HeapObjectAndCode, weak_objects_in_code, WeakObjectsInCode) \ + F(JSWeakRef, js_weak_refs, JSWeakRefs) \ + F(WeakCell, weak_cells, WeakCells) \ + F(SharedFunctionInfo, bytecode_flushing_candidates, \ + BytecodeFlushingCandidates) \ + F(JSFunction, flushed_js_functions, FlushedJSFunctions) + +class WeakObjects { + public: + template + using WeakObjectWorklist = Worklist; + +#define DECLARE_WORKLIST(Type, name, _) WeakObjectWorklist name; + WEAK_OBJECT_WORKLISTS(DECLARE_WORKLIST) +#undef DECLARE_WORKLIST + + void UpdateAfterScavenge(); + + private: +#define DECLARE_UPDATE_METHODS(Type, _, Name) \ + void Update##Name(WeakObjectWorklist&); + WEAK_OBJECT_WORKLISTS(DECLARE_UPDATE_METHODS) +#undef DECLARE_UPDATE_METHODS + +#ifdef DEBUG + template + bool ContainsYoungObjects(WeakObjectWorklist& worklist); +#endif +}; + +} // namespace internal +} // namespace v8 + +#endif // V8_HEAP_WEAK_OBJECT_WORKLISTS_H_