[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 <mlippautz@chromium.org>
Reviewed-by: Hannes Payer <hpayer@chromium.org>
Reviewed-by: Jaroslav Sevcik <jarin@chromium.org>
Commit-Queue: Ulan Degenbaev <ulan@chromium.org>
Cr-Commit-Position: refs/heads/master@{#57584}

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()) {

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);

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);

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->embedded_objects_cleared()) {
-      code->SetMarkedForDeoptimization("weak objects");
+      if (!code->marked_for_deoptimization()) {
-      code->InvalidateEmbeddedObjects(heap_);
+        code->SetMarkedForDeoptimization("weak objects");
-      have_code_to_deoptimize_ = true;
+        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(),

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);

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);
 }
 
 

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();

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, _)             \

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> 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<JSFunction> bar = Handle<JSFunction>::cast(v8::Utils::OpenHandle(
+        *v8::Local<v8::Function>::Cast(CcTest::global()
+                                           ->Get(context.local(), v8_str("bar"))
+                                           .ToLocalChecked())));
+    code = scope.CloseAndEscape(Handle<Code>(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<JSFunction> OptimizeDummyFunction(v8::Isolate* isolate,