From 6b55356d3a0e85c8bd4ed0280c01872effb125e0 Mon Sep 17 00:00:00 2001 From: Ulan Degenbaev Date: Wed, 14 Nov 2018 17:40:09 +0100 Subject: [PATCH] [heap] Decouple code deoptimization from clearing weak objects. This patch allows the deoptimizer to keep embedded pointers intact. Previously, the deoptimizer had to clear embedded pointers because the mark-compactor relied on the Code::marked_for_deoptimization flag to indicate whether the embedder pointers were cleared or not. This patch adds a new flag called Code::embedded_objects_cleared() and thus can correctly clear dead weak objects in deoptimized code. Bug: v8:8459 Change-Id: I6eb6ff3aa2182bc41730e0a249965f8d8c0525ce Reviewed-on: https://chromium-review.googlesource.com/c/1335943 Reviewed-by: Michael Lippautz Reviewed-by: Hannes Payer Reviewed-by: Jaroslav Sevcik Commit-Queue: Ulan Degenbaev Cr-Commit-Position: refs/heads/master@{#57584} --- src/deoptimizer.cc | 2 -- src/heap/heap.cc | 6 ----- src/heap/heap.h | 4 --- src/heap/mark-compact.cc | 16 +++++------- src/heap/mark-compact.h | 2 -- src/objects.cc | 3 ++- src/objects/code-inl.h | 14 ++++++++++ src/objects/code.h | 13 ++++++++-- test/cctest/heap/test-heap.cc | 48 +++++++++++++++++++++++++++++++++++ 9 files changed, 82 insertions(+), 26 deletions(-) diff --git a/src/deoptimizer.cc b/src/deoptimizer.cc index 0508c801e3..af1449ca78 100644 --- a/src/deoptimizer.cc +++ b/src/deoptimizer.cc @@ -332,8 +332,6 @@ void Deoptimizer::DeoptimizeMarkedCodeForContext(Context* context) { Object* next = code->next_code_link(); if (code->marked_for_deoptimization()) { - // Make sure that this object does not point to any garbage. - isolate->heap()->InvalidateCodeEmbeddedObjects(code); codes.insert(code); if (!prev.is_null()) { diff --git a/src/heap/heap.cc b/src/heap/heap.cc index 45ea91f4fb..297c7be59c 100644 --- a/src/heap/heap.cc +++ b/src/heap/heap.cc @@ -816,12 +816,6 @@ void Heap::ProcessPretenuringFeedback() { } } -void Heap::InvalidateCodeEmbeddedObjects(Code code) { - MemoryChunk* chunk = MemoryChunk::FromAddress(code->ptr()); - CodePageMemoryModificationScope modification_scope(chunk); - code->InvalidateEmbeddedObjects(this); -} - void Heap::InvalidateCodeDeoptimizationData(Code code) { MemoryChunk* chunk = MemoryChunk::FromAddress(code->ptr()); CodePageMemoryModificationScope modification_scope(chunk); diff --git a/src/heap/heap.h b/src/heap/heap.h index b8ad6b05f7..0f0c6d313d 100644 --- a/src/heap/heap.h +++ b/src/heap/heap.h @@ -869,10 +869,6 @@ class Heap { void SetConstructStubInvokeDeoptPCOffset(int pc_offset); void SetInterpreterEntryReturnPCOffset(int pc_offset); - // Invalidates references in the given {code} object that are directly - // embedded within the instruction stream. Mutates write-protected code. - void InvalidateCodeEmbeddedObjects(Code code); - // Invalidates references in the given {code} object that are referenced // transitively from the deoptimization data. Mutates write-protected code. void InvalidateCodeDeoptimizationData(Code code); diff --git a/src/heap/mark-compact.cc b/src/heap/mark-compact.cc index d49bf11c69..9ff0b47d27 100644 --- a/src/heap/mark-compact.cc +++ b/src/heap/mark-compact.cc @@ -1889,10 +1889,13 @@ void MarkCompactCollector::MarkDependentCodeForDeoptimization() { HeapObject* object = weak_object_in_code.first; Code code = weak_object_in_code.second; if (!non_atomic_marking_state()->IsBlackOrGrey(object) && - !code->marked_for_deoptimization()) { - code->SetMarkedForDeoptimization("weak objects"); - code->InvalidateEmbeddedObjects(heap_); - have_code_to_deoptimize_ = true; + !code->embedded_objects_cleared()) { + if (!code->marked_for_deoptimization()) { + code->SetMarkedForDeoptimization("weak objects"); + have_code_to_deoptimize_ = true; + } + code->ClearEmbeddedObjects(heap_); + DCHECK(code->embedded_objects_cleared()); } } } @@ -2686,11 +2689,6 @@ class EvacuationWeakObjectRetainer : public WeakObjectRetainer { } }; -// Return true if the given code is deoptimized or will be deoptimized. -bool MarkCompactCollector::WillBeDeoptimized(Code code) { - return code->is_optimized_code() && code->marked_for_deoptimization(); -} - void MarkCompactCollector::RecordLiveSlotsOnPage(Page* page) { EvacuateRecordOnlyVisitor visitor(heap()); LiveObjectVisitor::VisitBlackObjectsNoFail(page, non_atomic_marking_state(), diff --git a/src/heap/mark-compact.h b/src/heap/mark-compact.h index 9df5d075f3..0d595aea3f 100644 --- a/src/heap/mark-compact.h +++ b/src/heap/mark-compact.h @@ -715,8 +715,6 @@ class MarkCompactCollector final : public MarkCompactCollectorBase { explicit MarkCompactCollector(Heap* heap); ~MarkCompactCollector() override; - bool WillBeDeoptimized(Code code); - void ComputeEvacuationHeuristics(size_t area_size, int* target_fragmentation_percent, size_t* max_evacuated_bytes); diff --git a/src/objects.cc b/src/objects.cc index da44d4d383..7f195ff89f 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -14381,7 +14381,7 @@ void ObjectVisitor::VisitRelocInfo(RelocIterator* it) { } } -void Code::InvalidateEmbeddedObjects(Heap* heap) { +void Code::ClearEmbeddedObjects(Heap* heap) { HeapObject* undefined = ReadOnlyRoots(heap).undefined_value(); int mode_mask = RelocInfo::ModeMask(RelocInfo::EMBEDDED_OBJECT); for (RelocIterator it(*this, mode_mask); !it.done(); it.next()) { @@ -14390,6 +14390,7 @@ void Code::InvalidateEmbeddedObjects(Heap* heap) { it.rinfo()->set_target_object(heap, undefined, SKIP_WRITE_BARRIER); } } + set_embedded_objects_cleared(true); } diff --git a/src/objects/code-inl.h b/src/objects/code-inl.h index bdb14da48c..8dac1dd32c 100644 --- a/src/objects/code-inl.h +++ b/src/objects/code-inl.h @@ -512,6 +512,20 @@ void Code::set_marked_for_deoptimization(bool flag) { code_data_container()->set_kind_specific_flags(updated); } +bool Code::embedded_objects_cleared() const { + DCHECK(kind() == OPTIMIZED_FUNCTION); + int flags = code_data_container()->kind_specific_flags(); + return EmbeddedObjectsClearedField::decode(flags); +} + +void Code::set_embedded_objects_cleared(bool flag) { + DCHECK(kind() == OPTIMIZED_FUNCTION); + DCHECK_IMPLIES(flag, marked_for_deoptimization()); + int previous = code_data_container()->kind_specific_flags(); + int updated = EmbeddedObjectsClearedField::update(previous, flag); + code_data_container()->set_kind_specific_flags(updated); +} + bool Code::deopt_already_counted() const { DCHECK(kind() == OPTIMIZED_FUNCTION); int flags = code_data_container()->kind_specific_flags(); diff --git a/src/objects/code.h b/src/objects/code.h index 0fe65eed8c..e3a6ea3dd8 100644 --- a/src/objects/code.h +++ b/src/objects/code.h @@ -76,7 +76,9 @@ class Code : public HeapObjectPtr { // [relocation_info]: Code relocation information DECL_ACCESSORS(relocation_info, ByteArray) - void InvalidateEmbeddedObjects(Heap* heap); + + // This function should be called only from GC. + void ClearEmbeddedObjects(Heap* heap); // [deoptimization_data]: Array containing data for deopt. DECL_ACCESSORS(deoptimization_data, FixedArray) @@ -164,10 +166,16 @@ class Code : public HeapObjectPtr { inline void set_handler_table_offset(int offset); // [marked_for_deoptimization]: For kind OPTIMIZED_FUNCTION tells whether - // the code is going to be deoptimized because of dead embedded maps. + // the code is going to be deoptimized. inline bool marked_for_deoptimization() const; inline void set_marked_for_deoptimization(bool flag); + // [embedded_objects_cleared]: For kind OPTIMIZED_FUNCTION tells whether + // the embedded objects in the code marked for deoptimization were cleared. + // Note that embedded_objects_cleared() implies marked_for_deoptimization(). + inline bool embedded_objects_cleared() const; + inline void set_embedded_objects_cleared(bool flag); + // [deopt_already_counted]: For kind OPTIMIZED_FUNCTION tells whether // the code was already deoptimized. inline bool deopt_already_counted() const; @@ -416,6 +424,7 @@ class Code : public HeapObjectPtr { // KindSpecificFlags layout (STUB, BUILTIN and OPTIMIZED_FUNCTION) #define CODE_KIND_SPECIFIC_FLAGS_BIT_FIELDS(V, _) \ V(MarkedForDeoptimizationField, bool, 1, _) \ + V(EmbeddedObjectsClearedField, bool, 1, _) \ V(DeoptAlreadyCountedField, bool, 1, _) \ V(CanHaveWeakObjectsField, bool, 1, _) \ V(IsConstructStubField, bool, 1, _) \ diff --git a/test/cctest/heap/test-heap.cc b/test/cctest/heap/test-heap.cc index 4898849121..0fff9d7f1e 100644 --- a/test/cctest/heap/test-heap.cc +++ b/test/cctest/heap/test-heap.cc @@ -3860,6 +3860,7 @@ TEST(CellsInOptimizedCodeAreWeak) { } CHECK(code->marked_for_deoptimization()); + CHECK(code->embedded_objects_cleared()); } @@ -3902,6 +3903,7 @@ TEST(ObjectsInOptimizedCodeAreWeak) { } CHECK(code->marked_for_deoptimization()); + CHECK(code->embedded_objects_cleared()); } TEST(NewSpaceObjectsInOptimizedCode) { @@ -3962,6 +3964,52 @@ TEST(NewSpaceObjectsInOptimizedCode) { } CHECK(code->marked_for_deoptimization()); + CHECK(code->embedded_objects_cleared()); +} + +TEST(ObjectsInEagerlyDeoptimizedCodeAreWeak) { + if (FLAG_always_opt || !FLAG_opt) return; + FLAG_allow_natives_syntax = true; + CcTest::InitializeVM(); + Isolate* isolate = CcTest::i_isolate(); + v8::internal::Heap* heap = CcTest::heap(); + + if (!isolate->use_optimizer()) return; + HandleScope outer_scope(heap->isolate()); + Handle code; + { + LocalContext context; + HandleScope scope(heap->isolate()); + + CompileRun( + "function bar() {" + " return foo(1);" + "};" + "function foo(x) { with (x) { return 1 + x; } };" + "%NeverOptimizeFunction(foo);" + "bar();" + "bar();" + "bar();" + "%OptimizeFunctionOnNextCall(bar);" + "bar();" + "%DeoptimizeFunction(bar);"); + + Handle bar = Handle::cast(v8::Utils::OpenHandle( + *v8::Local::Cast(CcTest::global() + ->Get(context.local(), v8_str("bar")) + .ToLocalChecked()))); + code = scope.CloseAndEscape(Handle(bar->code(), isolate)); + } + + CHECK(code->marked_for_deoptimization()); + + // Now make sure that a gc should get rid of the function + for (int i = 0; i < 4; i++) { + CcTest::CollectAllGarbage(); + } + + CHECK(code->marked_for_deoptimization()); + CHECK(code->embedded_objects_cleared()); } static Handle OptimizeDummyFunction(v8::Isolate* isolate,