From bb54f99982d99db6e758af2be8030e716b2f23a1 Mon Sep 17 00:00:00 2001 From: Sigurd Schneider Date: Fri, 19 Oct 2018 11:20:52 +0000 Subject: [PATCH] Revert "[embedded] Share a single RelocInfo between all trampolines" This reverts commit 1bf6e73553e9648252eb22447c3e3ee69fd799ec. Reason for revert: Breaks nosnap builds: https://ci.chromium.org/p/v8/builders/luci.v8.ci/V8%20Linux%20-%20nosnap%20-%20debug/21209 Original change's description: > [embedded] Share a single RelocInfo between all trampolines > > Creates a single RelocInfo to be used by all builtin trampolines and > stores it as a root. All trampolines then substitute this for their > trampoline at generation time with DCHECKs to make sure it is > identical. > > Also forces all non-trampoline RelocInfo ByteArrays for builtins to be > generated into RO_SPACE. > > On x64, this results in the OLD_SPACE part of the startup snapshot > decreasing in size from 166096 to 131248 (-34848) bytes and RO_SPACE > (in the read-only snapshot) increasing from 31176 to 31248 (+72) bytes. > > Bug: v8:8295 > Change-Id: I69f4a899b738f2023ed42501c2b9797d34305b06 > Reviewed-on: https://chromium-review.googlesource.com/c/1276468 > Commit-Queue: Dan Elphick > Reviewed-by: Ulan Degenbaev > Reviewed-by: Jakob Gruber > Cr-Commit-Position: refs/heads/master@{#56811} TBR=ulan@chromium.org,jgruber@chromium.org,delphick@chromium.org Change-Id: I57239af6f3fc9c403977da0561b8fe32c1a758e7 No-Presubmit: true No-Tree-Checks: true No-Try: true Bug: v8:8295 Reviewed-on: https://chromium-review.googlesource.com/c/1291070 Reviewed-by: Sigurd Schneider Commit-Queue: Sigurd Schneider Cr-Commit-Position: refs/heads/master@{#56814} --- src/builtins/builtins.cc | 63 ++++++----------------------- src/builtins/builtins.h | 5 --- src/heap/factory.cc | 23 +---------- src/heap/setup-heap-internal.cc | 3 -- src/objects.cc | 4 +- src/objects/code-inl.h | 8 ---- src/objects/code.h | 5 --- src/roots.h | 5 +-- tools/v8heapconst.py | 71 ++++++++++++++++----------------- 9 files changed, 53 insertions(+), 134 deletions(-) diff --git a/src/builtins/builtins.cc b/src/builtins/builtins.cc index f3bd77cbd5..572ca773b5 100644 --- a/src/builtins/builtins.cc +++ b/src/builtins/builtins.cc @@ -12,7 +12,6 @@ #include "src/isolate.h" #include "src/macro-assembler.h" #include "src/objects-inl.h" -#include "src/objects/fixed-array.h" #include "src/visitors.h" namespace v8 { @@ -365,39 +364,6 @@ bool Builtins::IsWasmRuntimeStub(int index) { UNREACHABLE(); } -namespace { - -class OffHeapTrampolineGenerator { - public: - explicit OffHeapTrampolineGenerator(Isolate* isolate) - : isolate_(isolate), - masm_(isolate, buffer, kBufferSize, CodeObjectRequired::kYes) {} - - CodeDesc Generate(Address off_heap_entry) { - // Generate replacement code that simply tail-calls the off-heap code. - DCHECK(!masm_.has_frame()); - { - FrameScope scope(&masm_, StackFrame::NONE); - masm_.JumpToInstructionStream(off_heap_entry); - } - - CodeDesc desc; - masm_.GetCode(isolate_, &desc); - return desc; - } - - Handle CodeObject() { return masm_.CodeObject(); } - - private: - Isolate* isolate_; - // Enough to fit the single jmp. - static constexpr size_t kBufferSize = 256; - byte buffer[kBufferSize]; - MacroAssembler masm_; -}; - -} // namespace - // static Handle Builtins::GenerateOffHeapTrampolineFor(Isolate* isolate, Address off_heap_entry) { @@ -405,26 +371,21 @@ Handle Builtins::GenerateOffHeapTrampolineFor(Isolate* isolate, DCHECK_NOT_NULL(isolate->embedded_blob()); DCHECK_NE(0, isolate->embedded_blob_size()); - OffHeapTrampolineGenerator generator(isolate); - CodeDesc desc = generator.Generate(off_heap_entry); + constexpr size_t buffer_size = 256; // Enough to fit the single jmp. + byte buffer[buffer_size]; // NOLINT(runtime/arrays) - return isolate->factory()->NewCode(desc, Code::BUILTIN, - generator.CodeObject()); -} + // Generate replacement code that simply tail-calls the off-heap code. + MacroAssembler masm(isolate, buffer, buffer_size, CodeObjectRequired::kYes); + DCHECK(!masm.has_frame()); + { + FrameScope scope(&masm, StackFrame::NONE); + masm.JumpToInstructionStream(off_heap_entry); + } -// static -Handle Builtins::GenerateOffHeapTrampolineRelocInfo( - Isolate* isolate) { - OffHeapTrampolineGenerator generator(isolate); - // Generate a jump to a dummy address as we're not actually interested in the - // generated instruction stream. - CodeDesc desc = generator.Generate(kNullAddress); + CodeDesc desc; + masm.GetCode(isolate, &desc); - Handle reloc_info = - isolate->factory()->NewByteArray(desc.reloc_size, TENURED_READ_ONLY); - Code::CopyRelocInfoToByteArray(*reloc_info, desc); - - return reloc_info; + return isolate->factory()->NewCode(desc, Code::BUILTIN, masm.CodeObject()); } // static diff --git a/src/builtins/builtins.h b/src/builtins/builtins.h index a53303c365..9f404a0ac0 100644 --- a/src/builtins/builtins.h +++ b/src/builtins/builtins.h @@ -12,7 +12,6 @@ namespace v8 { namespace internal { -class ByteArray; class Callable; template class Handle; @@ -189,10 +188,6 @@ class Builtins { static Handle GenerateOffHeapTrampolineFor(Isolate* isolate, Address off_heap_entry); - // Generate the RelocInfo ByteArray that would be generated for an offheap - // trampoline. - static Handle GenerateOffHeapTrampolineRelocInfo(Isolate* isolate); - private: static void Generate_CallFunction(MacroAssembler* masm, ConvertReceiverMode mode); diff --git a/src/heap/factory.cc b/src/heap/factory.cc index b864a9f2e1..f38e8e27cb 100644 --- a/src/heap/factory.cc +++ b/src/heap/factory.cc @@ -2575,9 +2575,7 @@ MaybeHandle Factory::TryNewCode( uint32_t stub_key, bool is_turbofanned, int stack_slots, int safepoint_table_offset, int handler_table_offset) { // Allocate objects needed for code initialization. - Handle reloc_info = NewByteArray( - desc.reloc_size, - Builtins::IsBuiltinId(builtin_index) ? TENURED_READ_ONLY : TENURED); + Handle reloc_info = NewByteArray(desc.reloc_size, TENURED); Handle data_container = NewCodeDataContainer(0); Handle source_position_table = maybe_source_position_table.is_null() @@ -2627,9 +2625,7 @@ Handle Factory::NewCode( uint32_t stub_key, bool is_turbofanned, int stack_slots, int safepoint_table_offset, int handler_table_offset) { // Allocate objects needed for code initialization. - Handle reloc_info = NewByteArray( - desc.reloc_size, - Builtins::IsBuiltinId(builtin_index) ? TENURED_READ_ONLY : TENURED); + Handle reloc_info = NewByteArray(desc.reloc_size, TENURED); Handle data_container = NewCodeDataContainer(0); Handle source_position_table = maybe_source_position_table.is_null() @@ -2713,21 +2709,6 @@ Handle Factory::NewOffHeapTrampolineFor(Handle code, result->set_safepoint_table_offset(code->safepoint_table_offset()); } - // Replace the newly generated trampoline's RelocInfo ByteArray with the - // canonical one stored in the roots to avoid duplicating it for every single - // builtin. - ByteArray* canonical_reloc_info = - ReadOnlyRoots(isolate()).off_heap_trampoline_relocation_info(); -#ifdef DEBUG - // Verify that the contents are the same. - ByteArray* reloc_info = result->relocation_info(); - DCHECK_EQ(reloc_info->length(), canonical_reloc_info->length()); - for (int i = 0; i < reloc_info->length(); ++i) { - DCHECK_EQ(reloc_info->get(i), canonical_reloc_info->get(i)); - } -#endif - result->set_relocation_info(canonical_reloc_info); - return result; } diff --git a/src/heap/setup-heap-internal.cc b/src/heap/setup-heap-internal.cc index 3e127a0106..c0dfb7f00c 100644 --- a/src/heap/setup-heap-internal.cc +++ b/src/heap/setup-heap-internal.cc @@ -896,9 +896,6 @@ void Heap::CreateInitialObjects() { set_noscript_shared_function_infos(roots.empty_weak_array_list()); - set_off_heap_trampoline_relocation_info( - *Builtins::GenerateOffHeapTrampolineRelocInfo(isolate_)); - // Evaluate the hash values which will then be cached in the strings. isolate()->factory()->zero_string()->Hash(); isolate()->factory()->one_string()->Hash(); diff --git a/src/objects.cc b/src/objects.cc index eeb3c86685..1c094865f8 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -14404,7 +14404,9 @@ void Code::CopyFromNoFlush(Heap* heap, const CodeDesc& desc) { } // Copy reloc info. - CopyRelocInfoToByteArray(unchecked_relocation_info(), desc); + CopyBytes(relocation_start(), + desc.buffer + desc.buffer_size - desc.reloc_size, + static_cast(desc.reloc_size)); // Unbox handles and relocate. Assembler* origin = desc.origin; diff --git a/src/objects/code-inl.h b/src/objects/code-inl.h index e980cda646..34c9f2fc28 100644 --- a/src/objects/code-inl.h +++ b/src/objects/code-inl.h @@ -341,14 +341,6 @@ int Code::ExecutableSize() const { return raw_instruction_size() + Code::kHeaderSize; } -// static -void Code::CopyRelocInfoToByteArray(ByteArray* dest, const CodeDesc& desc) { - DCHECK_EQ(dest->length(), desc.reloc_size); - CopyBytes(dest->GetDataStartAddress(), - desc.buffer + desc.buffer_size - desc.reloc_size, - static_cast(desc.reloc_size)); -} - int Code::CodeSize() const { return SizeFor(body_size()); } Code::Kind Code::kind() const { diff --git a/src/objects/code.h b/src/objects/code.h index 26e8feff02..95d7b598ad 100644 --- a/src/objects/code.h +++ b/src/objects/code.h @@ -308,11 +308,6 @@ class Code : public HeapObject, public NeverReadOnlySpaceObject { // Migrate code from desc without flushing the instruction cache. void CopyFromNoFlush(Heap* heap, const CodeDesc& desc); - // Copy the RelocInfo portion of |desc| to |dest|. The ByteArray must be - // exactly the same size as the RelocInfo in |desc|. - static inline void CopyRelocInfoToByteArray(ByteArray* dest, - const CodeDesc& desc); - // Flushes the instruction cache for the executable instructions of this code // object. void FlushICache() const; diff --git a/src/roots.h b/src/roots.h index 2f6a87010d..4210e432e7 100644 --- a/src/roots.h +++ b/src/roots.h @@ -211,10 +211,7 @@ class RootVisitor; V(HeapNumber*, minus_zero_value, MinusZeroValue) \ V(HeapNumber*, minus_infinity_value, MinusInfinityValue) \ /* Marker for self-references during code-generation */ \ - V(HeapObject*, self_reference_marker, SelfReferenceMarker) \ - /* Canonical trampoline RelocInfo */ \ - V(ByteArray*, off_heap_trampoline_relocation_info, \ - OffHeapTrampolineRelocationInfo) + V(HeapObject*, self_reference_marker, SelfReferenceMarker) // Mutable roots that are known to be immortal immovable, for which we can // safely skip write barriers. diff --git a/tools/v8heapconst.py b/tools/v8heapconst.py index 34f724d638..d8acc8609b 100644 --- a/tools/v8heapconst.py +++ b/tools/v8heapconst.py @@ -296,41 +296,41 @@ KNOWN_MAPS = { ("RO_SPACE", 0x02699): (171, "Tuple2Map"), ("RO_SPACE", 0x02739): (173, "ArrayBoilerplateDescriptionMap"), ("RO_SPACE", 0x02a79): (161, "InterceptorInfoMap"), - ("RO_SPACE", 0x05049): (153, "AccessCheckInfoMap"), - ("RO_SPACE", 0x05099): (154, "AccessorInfoMap"), - ("RO_SPACE", 0x050e9): (155, "AccessorPairMap"), - ("RO_SPACE", 0x05139): (156, "AliasedArgumentsEntryMap"), - ("RO_SPACE", 0x05189): (157, "AllocationMementoMap"), - ("RO_SPACE", 0x051d9): (158, "AsyncGeneratorRequestMap"), - ("RO_SPACE", 0x05229): (159, "DebugInfoMap"), - ("RO_SPACE", 0x05279): (160, "FunctionTemplateInfoMap"), - ("RO_SPACE", 0x052c9): (162, "InterpreterDataMap"), - ("RO_SPACE", 0x05319): (163, "ModuleInfoEntryMap"), - ("RO_SPACE", 0x05369): (164, "ModuleMap"), - ("RO_SPACE", 0x053b9): (165, "ObjectTemplateInfoMap"), - ("RO_SPACE", 0x05409): (166, "PromiseCapabilityMap"), - ("RO_SPACE", 0x05459): (167, "PromiseReactionMap"), - ("RO_SPACE", 0x054a9): (168, "PrototypeInfoMap"), - ("RO_SPACE", 0x054f9): (169, "ScriptMap"), - ("RO_SPACE", 0x05549): (170, "StackFrameInfoMap"), - ("RO_SPACE", 0x05599): (172, "Tuple3Map"), - ("RO_SPACE", 0x055e9): (174, "WasmDebugInfoMap"), - ("RO_SPACE", 0x05639): (175, "WasmExportedFunctionDataMap"), - ("RO_SPACE", 0x05689): (176, "CallableTaskMap"), - ("RO_SPACE", 0x056d9): (177, "CallbackTaskMap"), - ("RO_SPACE", 0x05729): (178, "PromiseFulfillReactionJobTaskMap"), - ("RO_SPACE", 0x05779): (179, "PromiseRejectReactionJobTaskMap"), - ("RO_SPACE", 0x057c9): (180, "PromiseResolveThenableJobTaskMap"), - ("RO_SPACE", 0x05819): (181, "MicrotaskQueueMap"), - ("RO_SPACE", 0x05869): (182, "AllocationSiteWithWeakNextMap"), - ("RO_SPACE", 0x058b9): (182, "AllocationSiteWithoutWeakNextMap"), - ("RO_SPACE", 0x05909): (214, "LoadHandler1Map"), - ("RO_SPACE", 0x05959): (214, "LoadHandler2Map"), - ("RO_SPACE", 0x059a9): (214, "LoadHandler3Map"), - ("RO_SPACE", 0x059f9): (221, "StoreHandler0Map"), - ("RO_SPACE", 0x05a49): (221, "StoreHandler1Map"), - ("RO_SPACE", 0x05a99): (221, "StoreHandler2Map"), - ("RO_SPACE", 0x05ae9): (221, "StoreHandler3Map"), + ("RO_SPACE", 0x05031): (153, "AccessCheckInfoMap"), + ("RO_SPACE", 0x05081): (154, "AccessorInfoMap"), + ("RO_SPACE", 0x050d1): (155, "AccessorPairMap"), + ("RO_SPACE", 0x05121): (156, "AliasedArgumentsEntryMap"), + ("RO_SPACE", 0x05171): (157, "AllocationMementoMap"), + ("RO_SPACE", 0x051c1): (158, "AsyncGeneratorRequestMap"), + ("RO_SPACE", 0x05211): (159, "DebugInfoMap"), + ("RO_SPACE", 0x05261): (160, "FunctionTemplateInfoMap"), + ("RO_SPACE", 0x052b1): (162, "InterpreterDataMap"), + ("RO_SPACE", 0x05301): (163, "ModuleInfoEntryMap"), + ("RO_SPACE", 0x05351): (164, "ModuleMap"), + ("RO_SPACE", 0x053a1): (165, "ObjectTemplateInfoMap"), + ("RO_SPACE", 0x053f1): (166, "PromiseCapabilityMap"), + ("RO_SPACE", 0x05441): (167, "PromiseReactionMap"), + ("RO_SPACE", 0x05491): (168, "PrototypeInfoMap"), + ("RO_SPACE", 0x054e1): (169, "ScriptMap"), + ("RO_SPACE", 0x05531): (170, "StackFrameInfoMap"), + ("RO_SPACE", 0x05581): (172, "Tuple3Map"), + ("RO_SPACE", 0x055d1): (174, "WasmDebugInfoMap"), + ("RO_SPACE", 0x05621): (175, "WasmExportedFunctionDataMap"), + ("RO_SPACE", 0x05671): (176, "CallableTaskMap"), + ("RO_SPACE", 0x056c1): (177, "CallbackTaskMap"), + ("RO_SPACE", 0x05711): (178, "PromiseFulfillReactionJobTaskMap"), + ("RO_SPACE", 0x05761): (179, "PromiseRejectReactionJobTaskMap"), + ("RO_SPACE", 0x057b1): (180, "PromiseResolveThenableJobTaskMap"), + ("RO_SPACE", 0x05801): (181, "MicrotaskQueueMap"), + ("RO_SPACE", 0x05851): (182, "AllocationSiteWithWeakNextMap"), + ("RO_SPACE", 0x058a1): (182, "AllocationSiteWithoutWeakNextMap"), + ("RO_SPACE", 0x058f1): (214, "LoadHandler1Map"), + ("RO_SPACE", 0x05941): (214, "LoadHandler2Map"), + ("RO_SPACE", 0x05991): (214, "LoadHandler3Map"), + ("RO_SPACE", 0x059e1): (221, "StoreHandler0Map"), + ("RO_SPACE", 0x05a31): (221, "StoreHandler1Map"), + ("RO_SPACE", 0x05a81): (221, "StoreHandler2Map"), + ("RO_SPACE", 0x05ad1): (221, "StoreHandler3Map"), ("MAP_SPACE", 0x00139): (1057, "ExternalMap"), ("MAP_SPACE", 0x00189): (1073, "JSMessageObjectMap"), } @@ -384,7 +384,6 @@ KNOWN_OBJECTS = { ("RO_SPACE", 0x02af1): "MinusZeroValue", ("RO_SPACE", 0x02b01): "MinusInfinityValue", ("RO_SPACE", 0x02b11): "SelfReferenceMarker", - ("RO_SPACE", 0x02b69): "OffHeapTrampolineRelocationInfo", ("OLD_SPACE", 0x00139): "ArgumentsIteratorAccessor", ("OLD_SPACE", 0x001a9): "ArrayLengthAccessor", ("OLD_SPACE", 0x00219): "BoundFunctionLengthAccessor",