Revert "[heap] Make trampoline CodeDataContainers immutable and deduplicate them"

This reverts commit 652e32f9.

Reason for revert: speculative revert due to flaky test failures:
https://ci.chromium.org/p/v8/builders/ci/V8%20Linux%20-%20nosnap%20-%20debug/23920

Original change's description:
> [heap] Make trampoline CodeDataContainers immutable and deduplicate them
> 
> Moves all trampoline CodeDataContainers to read-only space, making them
> immutable. Containers with no 'kind specific flags' set or 'promise
> rejection' flag are deduplicated by replacing them with the new canonical
> CodeDataContainers roots.
> 
> This saves around 36KB from the snapshot.
> 
>     RO_SPACE  NEW_SPACE  OLD_SPACE  CODE_SPACE  MAP_SPACE  LO_SPACE
> old    32048          0     225944      149280      20240         0
> new    32120          0     189344      149280      20240         0
> 
> 
> Bug: v8:7464
> Change-Id: Iedd538a86311ef501cd88c90ec75e1308195762f
> Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1601257
> Commit-Queue: Maciej Goszczycki <goszczycki@google.com>
> Reviewed-by: Ulan Degenbaev <ulan@chromium.org>
> Reviewed-by: Dan Elphick <delphick@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#61378}

TBR=ulan@chromium.org,delphick@chromium.org,goszczycki@google.com

Change-Id: Ifaf9987bc3770f9e80701e8d011ab19da5c747ca
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: v8:7464
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1602877Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Adam Klein <adamk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#61388}

src/builtins/builtins.cc

@@ -291,8 +291,8 @@ constexpr int OffHeapTrampolineGenerator::kBufferSize;
 }  // namespace
 
 // static
-Handle<Code> Builtins::GenerateOffHeapTrampolineFor(
-    Isolate* isolate, Address off_heap_entry, int32_t kind_specfic_flags) {
+Handle<Code> Builtins::GenerateOffHeapTrampolineFor(Isolate* isolate,
+                                                    Address off_heap_entry) {
   DCHECK_NOT_NULL(isolate->embedded_blob());
   DCHECK_NE(0, isolate->embedded_blob_size());
 
@@ -301,7 +301,6 @@ Handle<Code> Builtins::GenerateOffHeapTrampolineFor(
 
   return Factory::CodeBuilder(isolate, desc, Code::BUILTIN)
       .set_self_reference(generator.CodeObject())
-      .set_read_only_data_container(kind_specfic_flags)
       .Build();
 }
 

src/builtins/builtins.h

@@ -172,8 +172,7 @@ class Builtins {
   // The result should not be used directly, but only from the related Factory
   // function.
   static Handle<Code> GenerateOffHeapTrampolineFor(Isolate* isolate,
-                                                   Address off_heap_entry,
-                                                   int32_t kind_specific_flags);
+                                                   Address off_heap_entry);
 
   // Generate the RelocInfo ByteArray that would be generated for an offheap
   // trampoline.

src/heap/factory.cc

@@ -86,29 +86,7 @@ MaybeHandle<Code> Factory::CodeBuilder::BuildInternal(
   // Allocate objects needed for code initialization.
   Handle<ByteArray> reloc_info =
       factory->NewByteArray(code_desc_.reloc_size, AllocationType::kOld);
-  Handle<CodeDataContainer> data_container;
-
-  // Use a canonical off-heap trampoline CodeDataContainer if possible.
-  const int32_t promise_rejection_flag =
-      Code::IsPromiseRejectionField::encode(true);
-  if (read_only_data_container_ &&
-      (kind_specific_flags_ == 0 ||
-       kind_specific_flags_ == promise_rejection_flag)) {
-    const ReadOnlyRoots roots(isolate_);
-    const auto canonical_code_data_container =
-        kind_specific_flags_ == 0
-            ? roots.trampoline_trivial_code_data_container_handle()
-            : roots.trampoline_promise_rejection_code_data_container_handle();
-    DCHECK_EQ(canonical_code_data_container->kind_specific_flags(),
-              kind_specific_flags_);
-    data_container = canonical_code_data_container;
-  } else {
-    data_container = factory->NewCodeDataContainer(
-        0, read_only_data_container_ ? AllocationType::kReadOnly
-                                     : AllocationType::kOld);
-    data_container->set_kind_specific_flags(kind_specific_flags_);
-  }
-
+  Handle<CodeDataContainer> data_container = factory->NewCodeDataContainer(0);
   Handle<Code> code;
   {
     int object_size = ComputeCodeObjectSize(code_desc_);
@@ -2706,10 +2684,10 @@ Handle<JSObject> Factory::NewExternal(void* value) {
   return external;
 }
 
-Handle<CodeDataContainer> Factory::NewCodeDataContainer(
-    int flags, AllocationType allocation) {
+Handle<CodeDataContainer> Factory::NewCodeDataContainer(int flags) {
   Handle<CodeDataContainer> data_container(
-      CodeDataContainer::cast(New(code_data_container_map(), allocation)),
+      CodeDataContainer::cast(
+          New(code_data_container_map(), AllocationType::kOld)),
       isolate());
   data_container->set_next_code_link(*undefined_value(), SKIP_WRITE_BARRIER);
   data_container->set_kind_specific_flags(flags);
@@ -2723,14 +2701,12 @@ Handle<Code> Factory::NewOffHeapTrampolineFor(Handle<Code> code,
   CHECK_NE(0, isolate()->embedded_blob_size());
   CHECK(Builtins::IsIsolateIndependentBuiltin(*code));
 
-  Handle<Code> result = Builtins::GenerateOffHeapTrampolineFor(
-      isolate(), off_heap_entry,
-      code->code_data_container()->kind_specific_flags());
-  // The CodeDataContainer should not be modified beyond this point since it's
-  // now possibly canonicalized.
+  Handle<Code> result =
+      Builtins::GenerateOffHeapTrampolineFor(isolate(), off_heap_entry);
 
   // The trampoline code object must inherit specific flags from the original
   // builtin (e.g. the safepoint-table offset). We set them manually here.
+
   {
     MemoryChunk* chunk = MemoryChunk::FromHeapObject(*result);
     CodePageMemoryModificationScope code_allocation(chunk);
@@ -2738,6 +2714,8 @@ Handle<Code> Factory::NewOffHeapTrampolineFor(Handle<Code> code,
     const bool set_is_off_heap_trampoline = true;
     const int stack_slots =
         code->has_safepoint_info() ? code->stack_slots() : 0;
+    result->code_data_container()->set_kind_specific_flags(
+        code->code_data_container()->kind_specific_flags());
     result->initialize_flags(code->kind(), code->has_unwinding_info(),
                              code->is_turbofanned(), stack_slots,
                              set_is_off_heap_trampoline);
@@ -2767,8 +2745,8 @@ Handle<Code> Factory::NewOffHeapTrampolineFor(Handle<Code> code,
 }
 
 Handle<Code> Factory::CopyCode(Handle<Code> code) {
-  Handle<CodeDataContainer> data_container = NewCodeDataContainer(
-      code->code_data_container()->kind_specific_flags(), AllocationType::kOld);
+  Handle<CodeDataContainer> data_container =
+      NewCodeDataContainer(code->code_data_container()->kind_specific_flags());
 
   Heap* heap = isolate()->heap();
   Handle<Code> new_code;

src/heap/factory.h

@@ -793,8 +793,7 @@ class V8_EXPORT_PRIVATE Factory {
   Handle<JSObject> NewExternal(void* value);
 
   // Creates a new CodeDataContainer for a Code object.
-  Handle<CodeDataContainer> NewCodeDataContainer(int flags,
-                                                 AllocationType allocation);
+  Handle<CodeDataContainer> NewCodeDataContainer(int flags);
 
   // Allocates a new code object and initializes it as the trampoline to the
   // given off-heap entry point.
@@ -958,44 +957,29 @@ class V8_EXPORT_PRIVATE Factory {
       self_reference_ = self_reference;
       return *this;
     }
-
     CodeBuilder& set_builtin_index(int32_t builtin_index) {
       builtin_index_ = builtin_index;
       return *this;
     }
-
     CodeBuilder& set_source_position_table(Handle<ByteArray> table) {
       DCHECK(!table.is_null());
       source_position_table_ = table;
       return *this;
     }
-
     CodeBuilder& set_deoptimization_data(
         Handle<DeoptimizationData> deopt_data) {
       DCHECK(!deopt_data.is_null());
       deoptimization_data_ = deopt_data;
       return *this;
     }
-
     CodeBuilder& set_immovable() {
       is_movable_ = false;
       return *this;
     }
-
     CodeBuilder& set_is_turbofanned() {
       is_turbofanned_ = true;
       return *this;
     }
-
-    // Indicates the CodeDataContainer should be allocated in read-only space.
-    // As an optimization, if the kind-specific flags match that of a canonical
-    // container, it will be used instead.
-    CodeBuilder& set_read_only_data_container(int32_t flags) {
-      read_only_data_container_ = true;
-      kind_specific_flags_ = flags;
-      return *this;
-    }
-
     CodeBuilder& set_stack_slots(int stack_slots) {
       stack_slots_ = stack_slots;
       return *this;
@@ -1010,11 +994,9 @@ class V8_EXPORT_PRIVATE Factory {
 
     MaybeHandle<Object> self_reference_;
     int32_t builtin_index_ = Builtins::kNoBuiltinId;
-    int32_t kind_specific_flags_ = 0;
     Handle<ByteArray> source_position_table_;
     Handle<DeoptimizationData> deoptimization_data_ =
         DeoptimizationData::Empty(isolate_);
-    bool read_only_data_container_ = false;
     bool is_movable_ = true;
     bool is_turbofanned_ = false;
     int stack_slots_ = 0;

src/heap/setup-heap-internal.cc

@@ -939,19 +939,9 @@ void Heap::CreateInitialObjects() {
 
   set_noscript_shared_function_infos(roots.empty_weak_array_list());
 
-  /* Canonical off-heap trampoline data */
   set_off_heap_trampoline_relocation_info(
       *Builtins::GenerateOffHeapTrampolineRelocInfo(isolate_));
 
-  set_trampoline_trivial_code_data_container(
-      *isolate()->factory()->NewCodeDataContainer(0,
-                                                  AllocationType::kReadOnly));
-
-  set_trampoline_promise_rejection_code_data_container(
-      *isolate()->factory()->NewCodeDataContainer(
-          Code::IsPromiseRejectionField::encode(true),
-          AllocationType::kReadOnly));
-
   // Evaluate the hash values which will then be cached in the strings.
   isolate()->factory()->zero_string()->Hash();
   isolate()->factory()->one_string()->Hash();

src/roots.h

@@ -216,13 +216,9 @@ class Symbol;
   V(HeapNumber, minus_infinity_value, MinusInfinityValue)                      \
   /* Marker for self-references during code-generation */                      \
   V(HeapObject, self_reference_marker, SelfReferenceMarker)                    \
-  /* Canonical off-heap trampoline data */                                     \
+  /* Canonical trampoline RelocInfo */                                         \
   V(ByteArray, off_heap_trampoline_relocation_info,                            \
     OffHeapTrampolineRelocationInfo)                                           \
-  V(CodeDataContainer, trampoline_trivial_code_data_container,                 \
-    TrampolineTrivialCodeDataContainer)                                        \
-  V(CodeDataContainer, trampoline_promise_rejection_code_data_container,       \
-    TrampolinePromiseRejectionCodeDataContainer)                               \
   /* Hash seed */                                                              \
   V(ByteArray, hash_seed, HashSeed)
 

tools/v8heapconst.py

@@ -306,49 +306,49 @@ KNOWN_MAPS = {
   ("read_only_space", 0x026e9): (98, "EnumCacheMap"),
   ("read_only_space", 0x02789): (115, "ArrayBoilerplateDescriptionMap"),
   ("read_only_space", 0x02ad9): (101, "InterceptorInfoMap"),
-  ("read_only_space", 0x05159): (89, "AccessCheckInfoMap"),
-  ("read_only_space", 0x051a9): (90, "AccessorInfoMap"),
-  ("read_only_space", 0x051f9): (91, "AccessorPairMap"),
-  ("read_only_space", 0x05249): (92, "AliasedArgumentsEntryMap"),
-  ("read_only_space", 0x05299): (93, "AllocationMementoMap"),
-  ("read_only_space", 0x052e9): (94, "AsmWasmDataMap"),
-  ("read_only_space", 0x05339): (95, "AsyncGeneratorRequestMap"),
-  ("read_only_space", 0x05389): (96, "ClassPositionsMap"),
-  ("read_only_space", 0x053d9): (97, "DebugInfoMap"),
-  ("read_only_space", 0x05429): (99, "FunctionTemplateInfoMap"),
-  ("read_only_space", 0x05479): (100, "FunctionTemplateRareDataMap"),
-  ("read_only_space", 0x054c9): (102, "InterpreterDataMap"),
-  ("read_only_space", 0x05519): (103, "ModuleInfoEntryMap"),
-  ("read_only_space", 0x05569): (104, "ModuleMap"),
-  ("read_only_space", 0x055b9): (105, "ObjectTemplateInfoMap"),
-  ("read_only_space", 0x05609): (106, "PromiseCapabilityMap"),
-  ("read_only_space", 0x05659): (107, "PromiseReactionMap"),
-  ("read_only_space", 0x056a9): (108, "PrototypeInfoMap"),
-  ("read_only_space", 0x056f9): (109, "ScriptMap"),
-  ("read_only_space", 0x05749): (110, "SourcePositionTableWithFrameCacheMap"),
-  ("read_only_space", 0x05799): (111, "StackFrameInfoMap"),
-  ("read_only_space", 0x057e9): (112, "StackTraceFrameMap"),
-  ("read_only_space", 0x05839): (113, "Tuple2Map"),
-  ("read_only_space", 0x05889): (114, "Tuple3Map"),
-  ("read_only_space", 0x058d9): (116, "WasmCapiFunctionDataMap"),
-  ("read_only_space", 0x05929): (117, "WasmDebugInfoMap"),
-  ("read_only_space", 0x05979): (118, "WasmExceptionTagMap"),
-  ("read_only_space", 0x059c9): (119, "WasmExportedFunctionDataMap"),
-  ("read_only_space", 0x05a19): (120, "CallableTaskMap"),
-  ("read_only_space", 0x05a69): (121, "CallbackTaskMap"),
-  ("read_only_space", 0x05ab9): (122, "PromiseFulfillReactionJobTaskMap"),
-  ("read_only_space", 0x05b09): (123, "PromiseRejectReactionJobTaskMap"),
-  ("read_only_space", 0x05b59): (124, "PromiseResolveThenableJobTaskMap"),
-  ("read_only_space", 0x05ba9): (125, "FinalizationGroupCleanupJobTaskMap"),
-  ("read_only_space", 0x05bf9): (126, "AllocationSiteWithWeakNextMap"),
-  ("read_only_space", 0x05c49): (126, "AllocationSiteWithoutWeakNextMap"),
-  ("read_only_space", 0x05c99): (161, "LoadHandler1Map"),
-  ("read_only_space", 0x05ce9): (161, "LoadHandler2Map"),
-  ("read_only_space", 0x05d39): (161, "LoadHandler3Map"),
-  ("read_only_space", 0x05d89): (169, "StoreHandler0Map"),
-  ("read_only_space", 0x05dd9): (169, "StoreHandler1Map"),
-  ("read_only_space", 0x05e29): (169, "StoreHandler2Map"),
-  ("read_only_space", 0x05e79): (169, "StoreHandler3Map"),
+  ("read_only_space", 0x05129): (89, "AccessCheckInfoMap"),
+  ("read_only_space", 0x05179): (90, "AccessorInfoMap"),
+  ("read_only_space", 0x051c9): (91, "AccessorPairMap"),
+  ("read_only_space", 0x05219): (92, "AliasedArgumentsEntryMap"),
+  ("read_only_space", 0x05269): (93, "AllocationMementoMap"),
+  ("read_only_space", 0x052b9): (94, "AsmWasmDataMap"),
+  ("read_only_space", 0x05309): (95, "AsyncGeneratorRequestMap"),
+  ("read_only_space", 0x05359): (96, "ClassPositionsMap"),
+  ("read_only_space", 0x053a9): (97, "DebugInfoMap"),
+  ("read_only_space", 0x053f9): (99, "FunctionTemplateInfoMap"),
+  ("read_only_space", 0x05449): (100, "FunctionTemplateRareDataMap"),
+  ("read_only_space", 0x05499): (102, "InterpreterDataMap"),
+  ("read_only_space", 0x054e9): (103, "ModuleInfoEntryMap"),
+  ("read_only_space", 0x05539): (104, "ModuleMap"),
+  ("read_only_space", 0x05589): (105, "ObjectTemplateInfoMap"),
+  ("read_only_space", 0x055d9): (106, "PromiseCapabilityMap"),
+  ("read_only_space", 0x05629): (107, "PromiseReactionMap"),
+  ("read_only_space", 0x05679): (108, "PrototypeInfoMap"),
+  ("read_only_space", 0x056c9): (109, "ScriptMap"),
+  ("read_only_space", 0x05719): (110, "SourcePositionTableWithFrameCacheMap"),
+  ("read_only_space", 0x05769): (111, "StackFrameInfoMap"),
+  ("read_only_space", 0x057b9): (112, "StackTraceFrameMap"),
+  ("read_only_space", 0x05809): (113, "Tuple2Map"),
+  ("read_only_space", 0x05859): (114, "Tuple3Map"),
+  ("read_only_space", 0x058a9): (116, "WasmCapiFunctionDataMap"),
+  ("read_only_space", 0x058f9): (117, "WasmDebugInfoMap"),
+  ("read_only_space", 0x05949): (118, "WasmExceptionTagMap"),
+  ("read_only_space", 0x05999): (119, "WasmExportedFunctionDataMap"),
+  ("read_only_space", 0x059e9): (120, "CallableTaskMap"),
+  ("read_only_space", 0x05a39): (121, "CallbackTaskMap"),
+  ("read_only_space", 0x05a89): (122, "PromiseFulfillReactionJobTaskMap"),
+  ("read_only_space", 0x05ad9): (123, "PromiseRejectReactionJobTaskMap"),
+  ("read_only_space", 0x05b29): (124, "PromiseResolveThenableJobTaskMap"),
+  ("read_only_space", 0x05b79): (125, "FinalizationGroupCleanupJobTaskMap"),
+  ("read_only_space", 0x05bc9): (126, "AllocationSiteWithWeakNextMap"),
+  ("read_only_space", 0x05c19): (126, "AllocationSiteWithoutWeakNextMap"),
+  ("read_only_space", 0x05c69): (161, "LoadHandler1Map"),
+  ("read_only_space", 0x05cb9): (161, "LoadHandler2Map"),
+  ("read_only_space", 0x05d09): (161, "LoadHandler3Map"),
+  ("read_only_space", 0x05d59): (169, "StoreHandler0Map"),
+  ("read_only_space", 0x05da9): (169, "StoreHandler1Map"),
+  ("read_only_space", 0x05df9): (169, "StoreHandler2Map"),
+  ("read_only_space", 0x05e49): (169, "StoreHandler3Map"),
   ("map_space", 0x00141): (1057, "ExternalMap"),
   ("map_space", 0x00191): (1073, "JSMessageObjectMap"),
 }
@@ -404,9 +404,7 @@ KNOWN_OBJECTS = {
   ("read_only_space", 0x02b61): "MinusInfinityValue",
   ("read_only_space", 0x02b71): "SelfReferenceMarker",
   ("read_only_space", 0x02bc9): "OffHeapTrampolineRelocationInfo",
-  ("read_only_space", 0x02be1): "TrampolineTrivialCodeDataContainer",
-  ("read_only_space", 0x02bf9): "TrampolinePromiseRejectionCodeDataContainer",
-  ("read_only_space", 0x02c11): "HashSeed",
+  ("read_only_space", 0x02be1): "HashSeed",
   ("old_space", 0x00141): "ArgumentsIteratorAccessor",
   ("old_space", 0x001b1): "ArrayLengthAccessor",
   ("old_space", 0x00221): "BoundFunctionLengthAccessor",