[heap] Don't age bytecode when GCing for devtools snapshot

When preparing to take a heap snapshot for the devtools, V8 uses
CollectAllAvailableGarbage, which runs 2 to 7 rounds of garbage
collection, depending on whether weak callbacks indicate that further
rounds might be beneficial. Depending on how many rounds of GC run,
varying amounts of bytecode and baseline code may be flushed, leading to
inconsistent behavior and underreporting the amount of memory used by
bytecode and baseline code. In this change, I propose that bytecode
should not increase in age during these collections, so that the
resulting snapshot is a better indication of actual memory usage.

Change-Id: I644be37833f85bb58e2e2fad5da62949cbdc9bef
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3182885Reviewed-by: Dominik Inführ <dinfuehr@chromium.org>
Commit-Queue: Seth Brenith <seth.brenith@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#77122}

src/heap/concurrent-marking.cc

@@ -88,11 +88,13 @@ class ConcurrentMarkingVisitor final
                            WeakObjects* weak_objects, Heap* heap,
                            unsigned mark_compact_epoch,
                            base::EnumSet<CodeFlushMode> code_flush_mode,
-                           bool embedder_tracing_enabled, bool is_forced_gc,
+                           bool embedder_tracing_enabled,
+                           bool should_keep_ages_unchanged,
                            MemoryChunkDataMap* memory_chunk_data)
       : MarkingVisitorBase(task_id, local_marking_worklists, weak_objects, heap,
                            mark_compact_epoch, code_flush_mode,
-                           embedder_tracing_enabled, is_forced_gc),
+                           embedder_tracing_enabled,
+                           should_keep_ages_unchanged),
         marking_state_(memory_chunk_data),
         memory_chunk_data_(memory_chunk_data) {}
 
@@ -370,11 +372,12 @@ StrongDescriptorArray ConcurrentMarkingVisitor::Cast(HeapObject object) {
 class ConcurrentMarking::JobTask : public v8::JobTask {
  public:
   JobTask(ConcurrentMarking* concurrent_marking, unsigned mark_compact_epoch,
-          base::EnumSet<CodeFlushMode> code_flush_mode, bool is_forced_gc)
+          base::EnumSet<CodeFlushMode> code_flush_mode,
+          bool should_keep_ages_unchanged)
       : concurrent_marking_(concurrent_marking),
         mark_compact_epoch_(mark_compact_epoch),
         code_flush_mode_(code_flush_mode),
-        is_forced_gc_(is_forced_gc) {}
+        should_keep_ages_unchanged_(should_keep_ages_unchanged) {}
 
   ~JobTask() override = default;
   JobTask(const JobTask&) = delete;
@@ -385,13 +388,13 @@ class ConcurrentMarking::JobTask : public v8::JobTask {
     if (delegate->IsJoiningThread()) {
       // TRACE_GC is not needed here because the caller opens the right scope.
       concurrent_marking_->Run(delegate, code_flush_mode_, mark_compact_epoch_,
-                               is_forced_gc_);
+                               should_keep_ages_unchanged_);
     } else {
       TRACE_GC_EPOCH(concurrent_marking_->heap_->tracer(),
                      GCTracer::Scope::MC_BACKGROUND_MARKING,
                      ThreadKind::kBackground);
       concurrent_marking_->Run(delegate, code_flush_mode_, mark_compact_epoch_,
-                               is_forced_gc_);
+                               should_keep_ages_unchanged_);
     }
   }
 
@@ -403,7 +406,7 @@ class ConcurrentMarking::JobTask : public v8::JobTask {
   ConcurrentMarking* concurrent_marking_;
   const unsigned mark_compact_epoch_;
   base::EnumSet<CodeFlushMode> code_flush_mode_;
-  const bool is_forced_gc_;
+  const bool should_keep_ages_unchanged_;
 };
 
 ConcurrentMarking::ConcurrentMarking(Heap* heap,
@@ -424,7 +427,8 @@ ConcurrentMarking::ConcurrentMarking(Heap* heap,
 
 void ConcurrentMarking::Run(JobDelegate* delegate,
                             base::EnumSet<CodeFlushMode> code_flush_mode,
-                            unsigned mark_compact_epoch, bool is_forced_gc) {
+                            unsigned mark_compact_epoch,
+                            bool should_keep_ages_unchanged) {
   size_t kBytesUntilInterruptCheck = 64 * KB;
   int kObjectsUntilInterrupCheck = 1000;
   uint8_t task_id = delegate->GetTaskId() + 1;
@@ -433,7 +437,7 @@ void ConcurrentMarking::Run(JobDelegate* delegate,
   ConcurrentMarkingVisitor visitor(
       task_id, &local_marking_worklists, weak_objects_, heap_,
       mark_compact_epoch, code_flush_mode,
-      heap_->local_embedder_heap_tracer()->InUse(), is_forced_gc,
+      heap_->local_embedder_heap_tracer()->InUse(), should_keep_ages_unchanged,
       &task_state->memory_chunk_data);
   NativeContextInferrer& native_context_inferrer =
       task_state->native_context_inferrer;
@@ -576,7 +580,7 @@ void ConcurrentMarking::ScheduleJob(TaskPriority priority) {
       priority, std::make_unique<JobTask>(
                     this, heap_->mark_compact_collector()->epoch(),
                     heap_->mark_compact_collector()->code_flush_mode(),
-                    heap_->is_current_gc_forced()));
+                    heap_->ShouldCurrentGCKeepAgesUnchanged()));
   DCHECK(job_handle_->IsValid());
 }
 

src/heap/concurrent-marking.h

@@ -106,7 +106,7 @@ class V8_EXPORT_PRIVATE ConcurrentMarking {
   };
   class JobTask;
   void Run(JobDelegate* delegate, base::EnumSet<CodeFlushMode> code_flush_mode,
-           unsigned mark_compact_epoch, bool is_forced_gc);
+           unsigned mark_compact_epoch, bool should_keep_ages_unchanged);
   size_t GetMaxConcurrency(size_t worker_count);
 
   std::unique_ptr<JobHandle> job_handle_;

src/heap/heap.cc

@@ -1684,6 +1684,8 @@ bool Heap::CollectGarbage(AllocationSpace space,
   is_current_gc_forced_ = gc_callback_flags & v8::kGCCallbackFlagForced ||
                           current_gc_flags_ & kForcedGC ||
                           force_gc_on_next_allocation_;
+  is_current_gc_for_heap_profiler_ =
+      gc_reason == GarbageCollectionReason::kHeapProfiler;
   if (force_gc_on_next_allocation_) force_gc_on_next_allocation_ = false;
 
   DevToolsTraceEventScope devtools_trace_event_scope(
@@ -1789,10 +1791,11 @@ bool Heap::CollectGarbage(AllocationSpace space,
         freed_global_handles +=
             PerformGarbageCollection(collector, gc_callback_flags);
       }
-      // Clear is_current_gc_forced now that the current GC is complete. Do this
-      // before GarbageCollectionEpilogue() since that could trigger another
-      // unforced GC.
+      // Clear flags describing the current GC now that the current GC is
+      // complete. Do this before GarbageCollectionEpilogue() since that could
+      // trigger another unforced GC.
       is_current_gc_forced_ = false;
+      is_current_gc_for_heap_profiler_ = false;
 
       {
         TRACE_GC(tracer(), GCTracer::Scope::HEAP_EXTERNAL_WEAK_GLOBAL_HANDLES);

src/heap/heap.h

@@ -1465,6 +1465,12 @@ class Heap {
 
   bool is_current_gc_forced() const { return is_current_gc_forced_; }
 
+  // Returns whether the currently in-progress GC should avoid increasing the
+  // ages on any objects that live for a set number of collections.
+  bool ShouldCurrentGCKeepAgesUnchanged() const {
+    return is_current_gc_forced_ || is_current_gc_for_heap_profiler_;
+  }
+
   // Returns the size of objects residing in non-new spaces.
   // Excludes external memory held by those objects.
   V8_EXPORT_PRIVATE size_t OldGenerationSizeOfObjects();
@@ -2452,6 +2458,7 @@ class Heap {
   std::unique_ptr<GlobalSafepoint> safepoint_;
 
   bool is_current_gc_forced_ = false;
+  bool is_current_gc_for_heap_profiler_ = false;
 
   ExternalStringTable external_string_table_;
 

src/heap/mark-compact.cc

@@ -559,7 +559,7 @@ void MarkCompactCollector::StartMarking() {
   marking_visitor_ = std::make_unique<MarkingVisitor>(
       marking_state(), local_marking_worklists(), weak_objects(), heap_,
       epoch(), code_flush_mode(), heap_->local_embedder_heap_tracer()->InUse(),
-      heap_->is_current_gc_forced());
+      heap_->ShouldCurrentGCKeepAgesUnchanged());
 // Marking bits are cleared by the sweeper.
 #ifdef VERIFY_HEAP
   if (FLAG_verify_heap) {

src/heap/mark-compact.h

@@ -377,11 +377,12 @@ class MainMarkingVisitor final
                      WeakObjects* weak_objects, Heap* heap,
                      unsigned mark_compact_epoch,
                      base::EnumSet<CodeFlushMode> code_flush_mode,
-                     bool embedder_tracing_enabled, bool is_forced_gc)
+                     bool embedder_tracing_enabled,
+                     bool should_keep_ages_unchanged)
       : MarkingVisitorBase<MainMarkingVisitor<MarkingState>, MarkingState>(
             kMainThreadTask, local_marking_worklists, weak_objects, heap,
             mark_compact_epoch, code_flush_mode, embedder_tracing_enabled,
-            is_forced_gc),
+            should_keep_ages_unchanged),
         marking_state_(marking_state),
         revisiting_object_(false) {}
 

src/heap/marking-visitor-inl.h

@@ -152,7 +152,7 @@ int MarkingVisitorBase<ConcreteVisitor, MarkingState>::VisitBytecodeArray(
   int size = BytecodeArray::BodyDescriptor::SizeOf(map, object);
   this->VisitMapPointer(object);
   BytecodeArray::BodyDescriptor::IterateBody(map, object, size, this);
-  if (!is_forced_gc_) {
+  if (!should_keep_ages_unchanged_) {
     object.MakeOlder();
   }
   return size;

src/heap/marking-visitor.h

@@ -106,7 +106,8 @@ class MarkingVisitorBase : public HeapVisitor<int, ConcreteVisitor> {
                      WeakObjects* weak_objects, Heap* heap,
                      unsigned mark_compact_epoch,
                      base::EnumSet<CodeFlushMode> code_flush_mode,
-                     bool is_embedder_tracing_enabled, bool is_forced_gc)
+                     bool is_embedder_tracing_enabled,
+                     bool should_keep_ages_unchanged)
       : HeapVisitor<int, ConcreteVisitor>(heap),
         local_marking_worklists_(local_marking_worklists),
         weak_objects_(weak_objects),
@@ -115,7 +116,7 @@ class MarkingVisitorBase : public HeapVisitor<int, ConcreteVisitor> {
         mark_compact_epoch_(mark_compact_epoch),
         code_flush_mode_(code_flush_mode),
         is_embedder_tracing_enabled_(is_embedder_tracing_enabled),
-        is_forced_gc_(is_forced_gc),
+        should_keep_ages_unchanged_(should_keep_ages_unchanged),
         is_shared_heap_(heap->IsShared()) {}
 
   V8_INLINE int VisitBytecodeArray(Map map, BytecodeArray object);
@@ -205,7 +206,7 @@ class MarkingVisitorBase : public HeapVisitor<int, ConcreteVisitor> {
   const unsigned mark_compact_epoch_;
   const base::EnumSet<CodeFlushMode> code_flush_mode_;
   const bool is_embedder_tracing_enabled_;
-  const bool is_forced_gc_;
+  const bool should_keep_ages_unchanged_;
   const bool is_shared_heap_;
 };