From 4becbe345fef51236402932c69bca4b67828fd0f Mon Sep 17 00:00:00 2001 From: Leszek Swirski Date: Tue, 16 May 2017 16:52:10 +0100 Subject: [PATCH] [ignition] Change --trace-ignition to a runtime flag Generate the code (extra runtime calls) for --trace-ignition support at compile time, based on a #define (similar to TRACE_MAPS). Then check for --trace-ignition at run-time when deciding whether to actually print anything. This should make --trace-ignition less painful to use. Note that --trace-igition is disabled by default, even on debug builds. It has to be enabled with the gn arg "v8_enable_trace_ignition=true" As a drive-by, TRACE_MAPS is renamed to V8_TRACE_MAPS, for consistency, and SFI unique index (needed both by --trace-ignition and --trace-maps) is cleaned up to be behind another #define. Change-Id: I8dd0c62d0e6b7ee9c75541d45eb729dc03acbee9 Reviewed-on: https://chromium-review.googlesource.com/506203 Commit-Queue: Leszek Swirski Reviewed-by: Ross McIlroy Cr-Commit-Position: refs/heads/master@{#45346} --- BUILD.gn | 15 +++++++-------- gypfiles/features.gypi | 2 +- gypfiles/toolchain.gypi | 2 +- src/bootstrapper.cc | 2 +- src/factory.cc | 2 +- src/flag-definitions.h | 4 +++- src/globals.h | 5 +++++ src/interpreter/interpreter-assembler.cc | 18 +++++++++--------- src/isolate.cc | 2 +- src/isolate.h | 4 ++-- src/objects-inl.h | 2 +- src/objects-printer.cc | 8 ++------ src/objects.cc | 22 ++++++++++------------ src/objects.h | 14 +++++++------- src/runtime/runtime-interpreter.cc | 15 ++++++++++++++- src/runtime/runtime.h | 11 +++++++++-- src/setup-isolate-deserialize.cc | 8 -------- 17 files changed, 74 insertions(+), 62 deletions(-) diff --git a/BUILD.gn b/BUILD.gn index e045239ee7..14c7a696e6 100644 --- a/BUILD.gn +++ b/BUILD.gn @@ -67,14 +67,14 @@ declare_args() { # Sets -dOBJECT_PRINT. v8_enable_object_print = "" - # Sets -dTRACE_MAPS. + # Sets -dV8_TRACE_MAPS. v8_enable_trace_maps = "" # Sets -dV8_ENABLE_CHECKS. v8_enable_v8_checks = "" - # Builds the snapshot with --trace-ignition - v8_trace_ignition = false + # Sets -dV8_TRACE_IGNITION. + v8_enable_trace_ignition = false # With post mortem support enabled, metadata is embedded into libv8 that # describes various parameters of the VM for use by debuggers. See @@ -228,7 +228,10 @@ config("features") { defines += [ "VERIFY_PREDICTABLE" ] } if (v8_enable_trace_maps) { - defines += [ "TRACE_MAPS" ] + defines += [ "V8_TRACE_MAPS" ] + } + if (v8_enable_trace_ignition) { + defines += [ "V8_TRACE_IGNITION" ] } if (v8_enable_v8_checks) { defines += [ "V8_ENABLE_CHECKS" ] @@ -732,10 +735,6 @@ action("run_mksnapshot") { ] } - if (v8_trace_ignition) { - args += [ "--trace-ignition" ] - } - if (v8_use_external_startup_data) { outputs += [ "$root_out_dir/snapshot_blob.bin" ] args += [ diff --git a/gypfiles/features.gypi b/gypfiles/features.gypi index 64de5c6e53..684be69b6f 100644 --- a/gypfiles/features.gypi +++ b/gypfiles/features.gypi @@ -90,7 +90,7 @@ 'defines': ['VERIFY_HEAP',], }], ['v8_trace_maps==1', { - 'defines': ['TRACE_MAPS',], + 'defines': ['V8_TRACE_MAPS',], }], ['v8_enable_verify_predictable==1', { 'defines': ['VERIFY_PREDICTABLE',], diff --git a/gypfiles/toolchain.gypi b/gypfiles/toolchain.gypi index 815070a508..6b82cfbd49 100644 --- a/gypfiles/toolchain.gypi +++ b/gypfiles/toolchain.gypi @@ -1245,7 +1245,7 @@ 'OBJECT_PRINT', 'VERIFY_HEAP', 'DEBUG', - 'TRACE_MAPS' + 'V8_TRACE_MAPS' ], 'conditions': [ ['OS=="linux" or OS=="freebsd" or OS=="openbsd" or OS=="netbsd" or \ diff --git a/src/bootstrapper.cc b/src/bootstrapper.cc index 7787eb4614..ffef57c44e 100644 --- a/src/bootstrapper.cc +++ b/src/bootstrapper.cc @@ -5117,7 +5117,7 @@ Genesis::Genesis( AddToWeakNativeContextList(*native_context()); isolate->set_context(*native_context()); isolate->counters()->contexts_created_by_snapshot()->Increment(); -#if TRACE_MAPS +#if V8_TRACE_MAPS if (FLAG_trace_maps) { Handle object_fun = isolate->object_function(); PrintF("[TraceMap: InitialMap map= %p SFI= %d_Object ]\n", diff --git a/src/factory.cc b/src/factory.cc index 606b6e4b54..3948eec3a7 100644 --- a/src/factory.cc +++ b/src/factory.cc @@ -2489,7 +2489,7 @@ Handle Factory::NewSharedFunctionInfo( FeedbackMetadata::New(isolate(), &empty_spec); share->set_feedback_metadata(*feedback_metadata, SKIP_WRITE_BARRIER); share->set_function_literal_id(FunctionLiteral::kIdTypeInvalid); -#if TRACE_MAPS +#if V8_SFI_HAS_UNIQUE_ID share->set_unique_id(isolate()->GetNextUniqueSharedFunctionInfoId()); #endif share->set_profiler_ticks(0); diff --git a/src/flag-definitions.h b/src/flag-definitions.h index db15562bcd..2a5d49f168 100644 --- a/src/flag-definitions.h +++ b/src/flag-definitions.h @@ -331,8 +331,10 @@ DEFINE_BOOL(print_bytecode, false, "print bytecode generated by ignition interpreter") DEFINE_STRING(print_bytecode_filter, "*", "filter for selecting which functions to print bytecode") +#ifdef V8_TRACE_IGNITION DEFINE_BOOL(trace_ignition, false, "trace the bytecodes executed by the ignition interpreter") +#endif DEFINE_BOOL(trace_ignition_codegen, false, "trace the codegen of ignition interpreter bytecode handlers") DEFINE_BOOL(trace_ignition_dispatches, false, @@ -932,7 +934,7 @@ DEFINE_BOOL(trace_prototype_users, false, "Trace updates to prototype user tracking") DEFINE_BOOL(use_verbose_printer, true, "allows verbose printing") DEFINE_BOOL(trace_for_in_enumerate, false, "Trace for-in enumerate slow-paths") -#if TRACE_MAPS +#if V8_TRACE_MAPS DEFINE_BOOL(trace_maps, false, "trace map creation") #endif diff --git a/src/globals.h b/src/globals.h index 3f2decca37..6d34ab9f0a 100644 --- a/src/globals.h +++ b/src/globals.h @@ -120,6 +120,11 @@ namespace internal { #define V8_CONCURRENT_MARKING 0 +// Some types of tracing require the SFI to store a unique ID. +#if defined(V8_TRACE_MAPS) || defined(V8_TRACE_IGNITION) +#define V8_SFI_HAS_UNIQUE_ID 1 +#endif + typedef uint8_t byte; typedef byte* Address; diff --git a/src/interpreter/interpreter-assembler.cc b/src/interpreter/interpreter-assembler.cc index 217fdc6a3b..b41b49d262 100644 --- a/src/interpreter/interpreter-assembler.cc +++ b/src/interpreter/interpreter-assembler.cc @@ -49,9 +49,9 @@ InterpreterAssembler::InterpreterAssembler(CodeAssemblerState* state, dispatch_table_.Bind( Parameter(InterpreterDispatchDescriptor::kDispatchTable)); - if (FLAG_trace_ignition) { - TraceBytecode(Runtime::kInterpreterTraceBytecodeEntry); - } +#ifdef V8_TRACE_IGNITION + TraceBytecode(Runtime::kInterpreterTraceBytecodeEntry); +#endif RegisterCallGenerationCallbacks([this] { CallPrologue(); }, [this] { CallEpilogue(); }); @@ -1012,9 +1012,9 @@ Node* InterpreterAssembler::Advance(int delta) { } Node* InterpreterAssembler::Advance(Node* delta, bool backward) { - if (FLAG_trace_ignition) { - TraceBytecode(Runtime::kInterpreterTraceBytecodeExit); - } +#ifdef V8_TRACE_IGNITION + TraceBytecode(Runtime::kInterpreterTraceBytecodeExit); +#endif Node* next_offset = backward ? IntPtrSub(BytecodeOffset(), delta) : IntPtrAdd(BytecodeOffset(), delta); bytecode_offset_.Bind(next_offset); @@ -1088,9 +1088,9 @@ void InterpreterAssembler::InlineStar() { bytecode_ = Bytecode::kStar; accumulator_use_ = AccumulatorUse::kNone; - if (FLAG_trace_ignition) { - TraceBytecode(Runtime::kInterpreterTraceBytecodeEntry); - } +#ifdef V8_TRACE_IGNITION + TraceBytecode(Runtime::kInterpreterTraceBytecodeEntry); +#endif StoreRegister(GetAccumulator(), BytecodeOperandReg(0)); DCHECK_EQ(accumulator_use_, Bytecodes::GetAccumulatorUse(bytecode_)); diff --git a/src/isolate.cc b/src/isolate.cc index de51ef567b..faa04848cf 100644 --- a/src/isolate.cc +++ b/src/isolate.cc @@ -2346,7 +2346,7 @@ Isolate::Isolate(bool enable_serializer) optimizing_compile_dispatcher_(NULL), stress_deopt_count_(0), next_optimization_id_(0), -#if TRACE_MAPS +#if V8_SFI_HAS_UNIQUE_ID next_unique_sfi_id_(0), #endif is_running_microtasks_(false), diff --git a/src/isolate.h b/src/isolate.h index 1031be1aa0..d65a1f373a 100644 --- a/src/isolate.h +++ b/src/isolate.h @@ -1167,7 +1167,7 @@ class Isolate { std::string GetTurboCfgFileName(); -#if TRACE_MAPS +#if V8_SFI_HAS_UNIQUE_ID int GetNextUniqueSharedFunctionInfoId() { return next_unique_sfi_id_++; } #endif @@ -1541,7 +1541,7 @@ class Isolate { int next_optimization_id_; -#if TRACE_MAPS +#if V8_SFI_HAS_UNIQUE_ID int next_unique_sfi_id_; #endif diff --git a/src/objects-inl.h b/src/objects-inl.h index 69598a497a..18bbc7a81f 100644 --- a/src/objects-inl.h +++ b/src/objects-inl.h @@ -5921,7 +5921,7 @@ ACCESSORS(SharedFunctionInfo, construct_stub, Code, kConstructStubOffset) ACCESSORS(SharedFunctionInfo, feedback_metadata, FeedbackMetadata, kFeedbackMetadataOffset) SMI_ACCESSORS(SharedFunctionInfo, function_literal_id, kFunctionLiteralIdOffset) -#if TRACE_MAPS +#if V8_SFI_HAS_UNIQUE_ID SMI_ACCESSORS(SharedFunctionInfo, unique_id, kUniqueIdOffset) #endif ACCESSORS(SharedFunctionInfo, instance_class_name, Object, diff --git a/src/objects-printer.cc b/src/objects-printer.cc index 60d81da775..2ea68863cf 100644 --- a/src/objects-printer.cc +++ b/src/objects-printer.cc @@ -1523,9 +1523,7 @@ void LayoutDescriptor::Print(std::ostream& os) { // NOLINT #endif // OBJECT_PRINT - -#if TRACE_MAPS - +#if V8_TRACE_MAPS void Name::NameShortPrint() { if (this->IsString()) { @@ -1556,9 +1554,7 @@ int Name::NameShortPrint(Vector str) { } } - -#endif // TRACE_MAPS - +#endif // V8_TRACE_MAPS #if defined(DEBUG) || defined(OBJECT_PRINT) // This method is only meant to be called from gdb for debugging purposes. diff --git a/src/objects.cc b/src/objects.cc index c3640d8acc..3873cff537 100644 --- a/src/objects.cc +++ b/src/objects.cc @@ -5699,7 +5699,7 @@ void JSObject::MigrateSlowToFast(Handle object, NotifyMapChange(old_map, new_map, isolate); -#if TRACE_MAPS +#if V8_TRACE_MAPS if (FLAG_trace_maps) { PrintF("[TraceMaps: SlowToFast from= %p to= %p reason= %s ]\n", reinterpret_cast(*old_map), reinterpret_cast(*new_map), @@ -8655,7 +8655,7 @@ Handle Map::Normalize(Handle fast_map, PropertyNormalizationMode mode, cache->Set(fast_map, new_map); isolate->counters()->maps_normalized()->Increment(); } -#if TRACE_MAPS +#if V8_TRACE_MAPS if (FLAG_trace_maps) { PrintF("[TraceMaps: Normalize from= %p to= %p reason= %s ]\n", reinterpret_cast(*fast_map), @@ -8814,8 +8814,7 @@ Handle Map::ShareDescriptor(Handle map, return result; } - -#if TRACE_MAPS +#if V8_TRACE_MAPS // static void Map::TraceTransition(const char* what, Map* from, Map* to, Name* name) { @@ -8840,8 +8839,7 @@ void Map::TraceAllTransitions(Map* map) { } } -#endif // TRACE_MAPS - +#endif // V8_TRACE_MAPS void Map::ConnectTransition(Handle parent, Handle child, Handle name, SimpleTransitionFlag flag) { @@ -8856,12 +8854,12 @@ void Map::ConnectTransition(Handle parent, Handle child, } if (parent->is_prototype_map()) { DCHECK(child->is_prototype_map()); -#if TRACE_MAPS +#if V8_TRACE_MAPS Map::TraceTransition("NoTransition", *parent, *child, *name); #endif } else { TransitionArray::Insert(parent, name, child, flag); -#if TRACE_MAPS +#if V8_TRACE_MAPS Map::TraceTransition("Transition", *parent, *child, *name); #endif } @@ -8893,7 +8891,7 @@ Handle Map::CopyReplaceDescriptors( } else { result->InitializeDescriptors(*descriptors, *layout_descriptor); } -#if TRACE_MAPS +#if V8_TRACE_MAPS if (FLAG_trace_maps && // Mirror conditions above that did not call ConnectTransition(). (map->is_prototype_map() || @@ -9093,7 +9091,7 @@ Handle Map::CopyForTransition(Handle map, const char* reason) { new_map->InitializeDescriptors(*new_descriptors, *new_layout_descriptor); } -#if TRACE_MAPS +#if V8_TRACE_MAPS if (FLAG_trace_maps) { PrintF("[TraceMaps: CopyForTransition from= %p to= %p reason= %s ]\n", reinterpret_cast(*map), reinterpret_cast(*new_map), @@ -9276,7 +9274,7 @@ Handle Map::TransitionToDataProperty(Handle map, Handle name, if (!maybe_map.ToHandle(&result)) { Isolate* isolate = name->GetIsolate(); const char* reason = "TooManyFastProperties"; -#if TRACE_MAPS +#if V8_TRACE_MAPS std::unique_ptr> buffer; if (FLAG_trace_maps) { ScopedVector name_buffer(100); @@ -12651,7 +12649,7 @@ void JSFunction::SetInitialMap(Handle function, Handle map, } function->set_prototype_or_initial_map(*map); map->SetConstructor(*function); -#if TRACE_MAPS +#if V8_TRACE_MAPS if (FLAG_trace_maps) { PrintF("[TraceMaps: InitialMap map= %p SFI= %d_%s ]\n", reinterpret_cast(*map), function->shared()->unique_id(), diff --git a/src/objects.h b/src/objects.h index ca84064c42..bcb4cd49b7 100644 --- a/src/objects.h +++ b/src/objects.h @@ -5120,7 +5120,7 @@ class Map: public HeapObject { // Returns true if given field is unboxed double. inline bool IsUnboxedDoubleField(FieldIndex index); -#if TRACE_MAPS +#if V8_TRACE_MAPS static void TraceTransition(const char* what, Map* from, Map* to, Name* name); static void TraceAllTransitions(Map* map); #endif @@ -5931,9 +5931,9 @@ class SharedFunctionInfo: public HeapObject { inline int function_literal_id() const; inline void set_function_literal_id(int value); -#if TRACE_MAPS - // [unique_id] - For --trace-maps purposes, an identifier that's persistent - // even if the GC moves this SharedFunctionInfo. +#if V8_SFI_HAS_UNIQUE_ID + // [unique_id] - For tracing purposes, an identifier that's persistent even if + // the GC moves this SharedFunctionInfo. inline int unique_id() const; inline void set_unique_id(int value); #endif @@ -6271,7 +6271,7 @@ class SharedFunctionInfo: public HeapObject { kFunctionIdentifierOffset + kPointerSize; static const int kFunctionLiteralIdOffset = kFeedbackMetadataOffset + kPointerSize; -#if TRACE_MAPS +#if V8_SFI_HAS_UNIQUE_ID static const int kUniqueIdOffset = kFunctionLiteralIdOffset + kPointerSize; static const int kLastPointerFieldOffset = kUniqueIdOffset; #else @@ -7842,7 +7842,7 @@ class Name: public HeapObject { DECLARE_CAST(Name) DECLARE_PRINTER(Name) -#if TRACE_MAPS +#if V8_TRACE_MAPS void NameShortPrint(); int NameShortPrint(Vector str); #endif @@ -7961,7 +7961,7 @@ class Symbol: public Name { private: const char* PrivateSymbolToName() const; -#if TRACE_MAPS +#if V8_TRACE_MAPS friend class Name; // For PrivateSymbolToName. #endif diff --git a/src/runtime/runtime-interpreter.cc b/src/runtime/runtime-interpreter.cc index 9f3897bf64..5889a477c3 100644 --- a/src/runtime/runtime-interpreter.cc +++ b/src/runtime/runtime-interpreter.cc @@ -34,6 +34,8 @@ RUNTIME_FUNCTION(Runtime_InterpreterNewClosure) { static_cast(pretenured_flag)); } +#ifdef V8_TRACE_IGNITION + namespace { void AdvanceToOffsetForTracing( @@ -109,17 +111,22 @@ void PrintRegisters(std::ostream& os, bool is_input, } // namespace RUNTIME_FUNCTION(Runtime_InterpreterTraceBytecodeEntry) { + if (!FLAG_trace_ignition) { + return isolate->heap()->undefined_value(); + } + SealHandleScope shs(isolate); DCHECK_EQ(3, args.length()); CONVERT_ARG_HANDLE_CHECKED(BytecodeArray, bytecode_array, 0); CONVERT_SMI_ARG_CHECKED(bytecode_offset, 1); CONVERT_ARG_HANDLE_CHECKED(Object, accumulator, 2); - OFStream os(stdout); int offset = bytecode_offset - BytecodeArray::kHeaderSize + kHeapObjectTag; interpreter::BytecodeArrayIterator bytecode_iterator(bytecode_array); AdvanceToOffsetForTracing(bytecode_iterator, offset); if (offset == bytecode_iterator.current_offset()) { + OFStream os(stdout); + // Print bytecode. const uint8_t* base_address = bytecode_array->GetFirstBytecodeAddress(); const uint8_t* bytecode_address = base_address + offset; @@ -137,6 +144,10 @@ RUNTIME_FUNCTION(Runtime_InterpreterTraceBytecodeEntry) { } RUNTIME_FUNCTION(Runtime_InterpreterTraceBytecodeExit) { + if (!FLAG_trace_ignition) { + return isolate->heap()->undefined_value(); + } + SealHandleScope shs(isolate); DCHECK_EQ(3, args.length()); CONVERT_ARG_HANDLE_CHECKED(BytecodeArray, bytecode_array, 0); @@ -160,6 +171,8 @@ RUNTIME_FUNCTION(Runtime_InterpreterTraceBytecodeExit) { return isolate->heap()->undefined_value(); } +#endif + RUNTIME_FUNCTION(Runtime_InterpreterAdvanceBytecodeOffset) { SealHandleScope shs(isolate); DCHECK_EQ(2, args.length()); diff --git a/src/runtime/runtime.h b/src/runtime/runtime.h index a67bee9735..ce150d395d 100644 --- a/src/runtime/runtime.h +++ b/src/runtime/runtime.h @@ -216,10 +216,17 @@ namespace internal { F(ForInFilter, 2, 1) \ F(ForInHasProperty, 2, 1) +#ifdef V8_TRACE_IGNITION +#define FOR_EACH_INTRINSIC_INTERPRETER_TRACE(F) \ + F(InterpreterTraceBytecodeEntry, 3, 1) \ + F(InterpreterTraceBytecodeExit, 3, 1) +#else +#define FOR_EACH_INTRINSIC_INTERPRETER_TRACE(F) +#endif + #define FOR_EACH_INTRINSIC_INTERPRETER(F) \ + FOR_EACH_INTRINSIC_INTERPRETER_TRACE(F) \ F(InterpreterNewClosure, 4, 1) \ - F(InterpreterTraceBytecodeEntry, 3, 1) \ - F(InterpreterTraceBytecodeExit, 3, 1) \ F(InterpreterAdvanceBytecodeOffset, 2, 1) #define FOR_EACH_INTRINSIC_FUNCTION(F) \ diff --git a/src/setup-isolate-deserialize.cc b/src/setup-isolate-deserialize.cc index 1effa0d995..a01bb5a3f8 100644 --- a/src/setup-isolate-deserialize.cc +++ b/src/setup-isolate-deserialize.cc @@ -20,14 +20,6 @@ void SetupIsolateDelegate::SetupBuiltins(Isolate* isolate, void SetupIsolateDelegate::SetupInterpreter( interpreter::Interpreter* interpreter, bool create_heap_objects) { -#ifdef V8_USE_SNAPSHOT - if (FLAG_trace_ignition || FLAG_trace_ignition_codegen || - FLAG_trace_ignition_dispatches) { - OFStream os(stdout); - os << "Warning: --trace-ignition-* flags must be passed at mksnapshot " - << "time or used with nosnapshot builds." << std::endl; - } -#endif DCHECK(interpreter->IsDispatchTableInitialized()); }