Optimize VM calls for record/replay opcodes, for #4

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.

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);

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<Script> script(Script::cast(shared->script()), isolate);
   CHECK(!RecordReplayIgnoreScript(*script));
 
-  if (!RecordReplayBytecodeAllowed()) {
-    Script::PositionInfo info;
-    Script::GetPositionInfo(script, shared->StartPosition(), &info, Script::WITH_OFFSET);
+  Script::PositionInfo info;
+  Script::GetPositionInfo(script, shared->StartPosition(), &info, Script::WITH_OFFSET);
 
-    if (script->name().IsUndefined()) {
-      recordreplay::Diagnostic("RecordReplayAssertExecutionProgress not allowed <none>:%d:%d",
-                               info.line + 1, info.column);
-    } else {
-      std::unique_ptr<char[]> name = String::cast(script->name()).ToCString();
-      recordreplay::Diagnostic("RecordReplayAssertExecutionProgress not allowed %s:%d:%d",
-                               name.get(), info.line + 1, info.column);
-    }
+  std::string name;
+  if (script->name().IsUndefined()) {
+    name = "<none>";
+  } else {
+    std::unique_ptr<char[]> name_raw = String::cast(script->name()).ToCString();
+    name = name_raw.get();
   }
-  CHECK(RecordReplayBytecodeAllowed());
-
-  RecordReplayIncrementProgressCounter();
 
-  if (ShouldEmitRecordReplayAssertValue()) {
-    Script::PositionInfo info;
-    Script::GetPositionInfo(script, shared->StartPosition(), &info, Script::WITH_OFFSET);
-
-    if (script->name().IsUndefined()) {
-      recordreplay::Assert("ExecutionProgress <none>:%d:%d",
-                          info.line + 1, info.column);
-    } else {
-      std::unique_ptr<char[]> name = String::cast(script->name()).ToCString();
-      recordreplay::Assert("ExecutionProgress %s:%d:%d",
-                          name.get(), info.line + 1, info.column);
-    }
+  if (!RecordReplayBytecodeAllowed()) {
+    recordreplay::Diagnostic("RecordReplayAssertExecutionProgress not allowed %s:%d:%d",
+                             name.c_str(), info.line + 1, info.column);
   }
+  CHECK(RecordReplayBytecodeAllowed());
+
+  recordreplay::Assert("ExecutionProgress %s:%d:%d",
+                       name.c_str(), info.line + 1, info.column);
 
   return ReadOnlyRoots(isolate).undefined_value();
 }
@@ -1104,7 +1108,7 @@ int RegisterAssertValueSite(const std::string& desc, int source_position) {
 extern std::string RecordReplayBasicValueContents(Handle<Object> value);
 
 RUNTIME_FUNCTION(Runtime_RecordReplayAssertValue) {
-  CHECK(ShouldEmitRecordReplayAssertValue());
+  CHECK(gRecordReplayAssertValues);
   CHECK(RecordReplayBytecodeAllowed());
 
   HandleScope scope(isolate);
@@ -1251,7 +1255,13 @@ static inline void OnInstrumentation(Isolate* isolate,
                          site.bytecode_offset_);
 }
 
+extern bool gRecordReplayInstrumentationEnabled;
+
 RUNTIME_FUNCTION(Runtime_RecordReplayInstrumentation) {
+  if (!gRecordReplayInstrumentationEnabled) {
+    return ReadOnlyRoots(isolate).undefined_value();
+  }
+
   HandleScope scope(isolate);
   DCHECK_EQ(2, args.length());
   CONVERT_ARG_HANDLE_CHECKED(JSFunction, function, 0);
@@ -1280,7 +1290,9 @@ RUNTIME_FUNCTION(Runtime_RecordReplayInstrumentationGenerator) {
   CHECK(!gCurrentGeneratorId);
   gCurrentGeneratorId = RecordReplayObjectId(generator_object);
 
-  OnInstrumentation(isolate, function, index);
+  if (!gRecordReplayInstrumentationEnabled) {
+    OnInstrumentation(isolate, function, index);
+  }
 
   gCurrentGeneratorId = 0;
 

src/runtime/runtime-internal.cc

@@ -171,7 +171,8 @@ RUNTIME_FUNCTION(Runtime_ThrowInvalidTypedArrayAlignment) {
 }
 
 extern void RecordReplayOnExceptionUnwind(Isolate* isolate);
-extern uint64_t* RecordReplayProgressCounter();
+
+extern uint64_t* gProgressCounter;
 
 static uint64_t gLastExceptionUnwindProgress;
 
@@ -189,9 +190,9 @@ RUNTIME_FUNCTION(Runtime_UnwindAndFindExceptionHandler) {
   // frame unwind instead of an initial throw and we can ignore it.
   if (recordreplay::IsRecordingOrReplaying() &&
       IsMainThread() &&
-      *RecordReplayProgressCounter() != gLastExceptionUnwindProgress) {
+      *gProgressCounter != gLastExceptionUnwindProgress) {
     RecordReplayOnExceptionUnwind(isolate);
-    gLastExceptionUnwindProgress = *RecordReplayProgressCounter();
+    gLastExceptionUnwindProgress = *gProgressCounter;
   }
 
   return isolate->UnwindAndFindHandler();