Skip to content

Commit

Permalink
Disable running the preamble during replay of trace
Browse files Browse the repository at this point in the history
Summary: The preamble of activity should be skipped when running from a replay of a trace. This is because that activity will already be in the trace itself, so executing it again would result in duplicated trace records.

Reviewed By: neildhar

Differential Revision: D37662040

fbshipit-source-id: 3722cb332f9df21467a99430070963c56f166239
  • Loading branch information
Michael Anthony Leon authored and facebook-github-bot committed Jul 14, 2022
1 parent cc48db8 commit c9dd444
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 13 deletions.
14 changes: 8 additions & 6 deletions API/hermes/TracingRuntime.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ TracingRuntime::TracingRuntime(
std::unique_ptr<llvh::raw_ostream> traceStream)
: RuntimeDecorator<jsi::Runtime>(*runtime),
runtime_(std::move(runtime)),
trace_(globalID, conf, std::move(traceStream)) {}
trace_(globalID, conf, std::move(traceStream)),
numPreambleRecords_(0) {}

void TracingRuntime::replaceNondeterministicFuncs() {
insertHostForwarder({"Math", "random"});
Expand Down Expand Up @@ -736,9 +737,7 @@ TracingHermesRuntime::TracingHermesRuntime(
runtimeConfig,
std::move(traceStream),
std::move(commitAction),
std::move(rollbackAction)) {
replaceNondeterministicFuncs();
}
std::move(rollbackAction)) {}

TracingHermesRuntime::~TracingHermesRuntime() {
if (crashCallbackKey_) {
Expand Down Expand Up @@ -870,10 +869,13 @@ static std::unique_ptr<TracingHermesRuntime> makeTracingHermesRuntimeImpl(
std::move(traceStream),
commitAction,
rollbackAction);
// In non-replay executions, add the __nativeRecordTraceMarker function.
// In replay executions, this will be simulated from the trace.
if (!forReplay) {
// In non-replay executions, add the __nativeRecordTraceMarker function.
// In replay executions, this will be simulated from the trace.
addRecordMarker(*ret);
// In replay executions, the trace will contain the instructions for
// replacing the nondeterministic functions.
ret->replaceNondeterministicFuncs();
}
return ret;
}
Expand Down
7 changes: 3 additions & 4 deletions unittests/API/SynthTraceSerializationTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -350,24 +350,23 @@ TEST_F(SynthTraceSerializationTest, FullTrace) {

auto records = optTrace.getPropertyAsObject(*rt, "trace").asArray(*rt);

auto offset = rt->getNumPreambleRecordsForTest();
auto record = records.getValueAtIndex(*rt, 0 + offset).asObject(*rt);
auto record = records.getValueAtIndex(*rt, 0).asObject(*rt);
EXPECT_EQ(
"CreateObjectRecord",
record.getProperty(*rt, "type").asString(*rt).utf8(*rt));
EXPECT_TRUE(record.getProperty(*rt, "time").isNumber());
EXPECT_EQ(objID, record.getProperty(*rt, "objID").asNumber());

// The obj.getProperty(*rt, "a") creates a string primitive for "a".
record = records.getValueAtIndex(*rt, 1 + offset).asObject(*rt);
record = records.getValueAtIndex(*rt, 1).asObject(*rt);
EXPECT_EQ(
"CreateStringRecord",
record.getProperty(*rt, "type").asString(*rt).utf8(*rt));
EXPECT_TRUE(record.getProperty(*rt, "time").isNumber());
EXPECT_TRUE(record.getProperty(*rt, "objID").isNumber());
auto stringID = record.getProperty(*rt, "objID").asNumber();

record = records.getValueAtIndex(*rt, 2 + offset).asObject(*rt);
record = records.getValueAtIndex(*rt, 2).asObject(*rt);
EXPECT_EQ(
"GetPropertyRecord",
record.getProperty(*rt, "type").asString(*rt).utf8(*rt));
Expand Down
11 changes: 8 additions & 3 deletions unittests/API/SynthTraceTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -64,9 +64,8 @@ struct SynthTraceTest : public ::testing::Test {
}

llvh::ArrayRef<std::unique_ptr<SynthTrace::Record>> getRecords() {
auto n = rt->getNumPreambleRecordsForTest();
auto &recs = rt->trace().records();
return llvh::makeArrayRef(recs.data() + n, recs.size() - n);
return llvh::makeArrayRef(recs.data(), recs.size());
}
};

Expand Down Expand Up @@ -1003,7 +1002,7 @@ struct SynthTraceReplayTest : public ::testing::Test {
config,
/* traceStream */
std::make_unique<llvh::raw_string_ostream>(traceResult),
/* forReplay */ true)) {}
/* forReplay */ false)) {}

SynthTraceReplayTest()
: SynthTraceReplayTest(::hermes::vm::RuntimeConfig::Builder()
Expand All @@ -1020,6 +1019,12 @@ struct SynthTraceReplayTest : public ::testing::Test {
jsi::Value eval(jsi::Runtime &rt, const char *code) {
return rt.global().getPropertyAsFunction(rt, "eval").call(rt, code);
}

llvh::ArrayRef<std::unique_ptr<SynthTrace::Record>> getRecords() {
auto n = traceRt->getNumPreambleRecordsForTest();
auto &recs = traceRt->trace().records();
return llvh::makeArrayRef(recs.data() + n, recs.size() - n);
}
};

TEST_F(SynthTraceReplayTest, CreateObjectReplay) {
Expand Down

0 comments on commit c9dd444

Please sign in to comment.