From 740a5dd5b4bfec3a7382d97ccebfd5cbe11b83a3 Mon Sep 17 00:00:00 2001 From: Brian Hackett Date: Mon, 3 May 2021 10:10:54 -0600 Subject: [PATCH] Optimize VM calls for record/replay opcodes, for #4 --- src/api/api.cc | 25 ++++--- src/interpreter/bytecode-array-builder.cc | 10 ++- src/runtime/runtime-debug.cc | 86 +++++++++++++---------- src/runtime/runtime-internal.cc | 7 +- 4 files changed, 74 insertions(+), 54 deletions(-) diff --git a/src/api/api.cc b/src/api/api.cc index 1a46241eacd7..09845e3cb8ec 100644 --- a/src/api/api.cc +++ b/src/api/api.cc @@ -9936,7 +9936,6 @@ static void (*gRecordReplayAssert)(const char*, va_list); static void (*gRecordReplayAssertBytes)(const char* why, const void* ptr, size_t nbytes); static void (*gRecordReplayBytes)(const char* why, void* buf, size_t size); static uintptr_t (*gRecordReplayValue)(const char* why, uintptr_t v); -static uint64_t* (*gRecordReplayProgressCounter)(); static bool (*gRecordReplayAreEventsDisallowed)(); static void (*gRecordReplayBeginPassThroughEvents)(); static void (*gRecordReplayEndPassThroughEvents)(); @@ -10011,11 +10010,13 @@ void RecordReplayOnExceptionUnwind(Isolate* isolate) { } } -static bool gHasCheckpoint; +uint64_t* gProgressCounter; -uint64_t* RecordReplayProgressCounter() { - CHECK(gHasCheckpoint); - return gRecordReplayProgressCounter(); +bool gRecordReplayInstrumentationEnabled; + +void RecordReplayChangeInstrument(bool enabled) { + CHECK(!enabled || recordreplay::IsReplaying()); + gRecordReplayInstrumentationEnabled = enabled; } void RecordReplayInstrument(const char* kind, const char* function, int offset) { @@ -10027,10 +10028,6 @@ extern void ClearPauseDataCallback(); bool gRecordReplayAssertValues; -bool ShouldEmitRecordReplayAssertValue() { - return gRecordReplayAssertValues; -} - // Only finish recordings if there were interesting sources loaded // into the process. static char* gRecordReplayInterestingSource; @@ -10242,7 +10239,6 @@ void recordreplay::NewCheckpoint() { // needed to process commands which we might get from the driver. if (IsRecordingOrReplaying() && IsMainThread() && internal::gDefaultContext) { gRecordReplayNewCheckpoint(); - internal::gHasCheckpoint = true; } } @@ -10532,7 +10528,6 @@ void recordreplay::SetRecordingOrReplaying(void* handle) { RecordReplayLoadSymbol(handle, "RecordReplayBytes", gRecordReplayBytes); RecordReplayLoadSymbol(handle, "RecordReplayValue", gRecordReplayValue); RecordReplayLoadSymbol(handle, "RecordReplayOnInstrument", gRecordReplayOnInstrument); - RecordReplayLoadSymbol(handle, "RecordReplayProgressCounter", gRecordReplayProgressCounter); RecordReplayLoadSymbol(handle, "RecordReplayAreEventsDisallowed", gRecordReplayAreEventsDisallowed); RecordReplayLoadSymbol(handle, "RecordReplayBeginPassThroughEvents", gRecordReplayBeginPassThroughEvents); RecordReplayLoadSymbol(handle, "RecordReplayEndPassThroughEvents", gRecordReplayEndPassThroughEvents); @@ -10573,6 +10568,14 @@ void recordreplay::SetRecordingOrReplaying(void* handle) { gFinishRecordingOrderedLockId = (int)CreateOrderedLock("FinishRecording"); + uint64_t* (*getProgressCounter)(); + RecordReplayLoadSymbol(handle, "RecordReplayProgressCounter", getProgressCounter); + internal::gProgressCounter = getProgressCounter(); + + void (*setChangeInstrumentCallback)(void (*callback)(bool wantInstrumentation)); + RecordReplayLoadSymbol(handle, "RecordReplaySetChangeInstrumentCallback", setChangeInstrumentCallback); + setChangeInstrumentCallback(internal::RecordReplayChangeInstrument); + internal::gRecordReplayAssertValues = !!getenv("RECORD_REPLAY_JS_ASSERTS"); // Set flags to disable non-deterministic posting of tasks to other threads. diff --git a/src/interpreter/bytecode-array-builder.cc b/src/interpreter/bytecode-array-builder.cc index 8958db9474db..1e151835b5eb 100644 --- a/src/interpreter/bytecode-array-builder.cc +++ b/src/interpreter/bytecode-array-builder.cc @@ -22,7 +22,7 @@ namespace internal { extern int RegisterAssertValueSite(const std::string& desc, int source_position); extern int RegisterInstrumentationSite(const char* kind, int source_position, int bytecode_offset); -extern bool ShouldEmitRecordReplayAssertValue(); +extern bool gRecordReplayAssertValues; namespace interpreter { @@ -1370,7 +1370,7 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::RecordReplayIncExecutionProgressCoun } BytecodeArrayBuilder& BytecodeArrayBuilder::RecordReplayAssertValue(const std::string& desc) { - if (emit_record_replay_opcodes_ && ShouldEmitRecordReplayAssertValue()) { + if (emit_record_replay_opcodes_ && gRecordReplayAssertValues) { int index = RegisterAssertValueSite(desc, most_recent_source_position_); OutputRecordReplayAssertValue(index); } @@ -1379,7 +1379,8 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::RecordReplayAssertValue(const std::s BytecodeArrayBuilder& BytecodeArrayBuilder::RecordReplayInstrumentation(const char* kind, int source_position) { - if (emit_record_replay_opcodes_) { + // Instrumentation opcodes aren't needed when recording. + if (emit_record_replay_opcodes_ && recordreplay::IsReplaying()) { int bytecode_offset = bytecode_array_writer_.size(); int index = RegisterInstrumentationSite(kind, source_position, bytecode_offset); OutputRecordReplayInstrumentation(index); @@ -1389,6 +1390,9 @@ BytecodeArrayBuilder& BytecodeArrayBuilder::RecordReplayInstrumentation(const ch BytecodeArrayBuilder& BytecodeArrayBuilder::RecordReplayInstrumentationGenerator( const char* kind, Register generator_object) { + // Even though instrumentation opcodes aren't needed when recording, we still + // need to emit InstrumentationGenerator opcodes so that generator objects + // will be associated with IDs at consistent points. if (emit_record_replay_opcodes_) { int bytecode_offset = bytecode_array_writer_.size(); int index = RegisterInstrumentationSite(kind, kNoSourcePosition, bytecode_offset); diff --git a/src/runtime/runtime-debug.cc b/src/runtime/runtime-debug.cc index 31b391723c4f..8a157666ca7a 100644 --- a/src/runtime/runtime-debug.cc +++ b/src/runtime/runtime-debug.cc @@ -960,18 +960,15 @@ RUNTIME_FUNCTION(Runtime_ProfileCreateSnapshotDataBlob) { return ReadOnlyRoots(isolate).undefined_value(); } -extern uint64_t* RecordReplayProgressCounter(); +extern uint64_t* gProgressCounter; +extern bool gRecordReplayAssertValues; -static inline void RecordReplayIncrementProgressCounter() { - // Note: The counter can be null, depending on the thread. - uint64_t* counter = RecordReplayProgressCounter(); - if (counter) { - ++*counter; - } -} +// Define this to check preconditions for using record/replay opcodes. +//#define RECORD_REPLAY_CHECK_OPCODES + +#ifdef RECORD_REPLAY_CHECK_OPCODES extern bool RecordReplayIgnoreScript(Script script); -extern bool ShouldEmitRecordReplayAssertValue(); extern "C" bool V8RecordReplayHasDivergedFromRecording(); @@ -980,7 +977,25 @@ static inline bool RecordReplayBytecodeAllowed() { && (!recordreplay::AreEventsDisallowed() || V8RecordReplayHasDivergedFromRecording()); } +#else // !RECORD_REPLAY_CHECK_OPCODES + +static inline bool RecordReplayIgnoreScript(Script script) { + return false; +} + +static inline bool RecordReplayBytecodeAllowed() { + return true; +} + +#endif // !RECORD_REPLAY_CHECK_OPCODES + RUNTIME_FUNCTION(Runtime_RecordReplayAssertExecutionProgress) { + ++*gProgressCounter; + + if (!gRecordReplayAssertValues) { + return ReadOnlyRoots(isolate).undefined_value(); + } + HandleScope scope(isolate); DCHECK_EQ(1, args.length()); CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 0); @@ -989,36 +1004,25 @@ RUNTIME_FUNCTION(Runtime_RecordReplayAssertExecutionProgress) { Handle