Skip to content

Commit

Permalink
Remove unused values
Browse files Browse the repository at this point in the history
Summary:
Original Author: mattblagden@meta.com
Original Git: 34d04dd
Original Reviewed By: neildhar
Original Revision: D59174037

Now that traces have been re-recorded, `GetProperty`,
`GetPropertyNames`, and `ArrayRead` recordings no longer populate their
return value fields. Remove them.

Reviewed By: neildhar

Differential Revision: D59403428

fbshipit-source-id: 77bd24340e924f432d9f20d4288692827253fe90
  • Loading branch information
Matt Blagden authored and facebook-github-bot committed Jul 9, 2024
1 parent 8a129dc commit 677377d
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 84 deletions.
59 changes: 11 additions & 48 deletions API/hermes/SynthTrace.h
Original file line number Diff line number Diff line change
Expand Up @@ -746,25 +746,24 @@ class SynthTrace {
#ifdef HERMESVM_API_TRACE_DEBUG
std::string propNameDbg_;
#endif
/// Returned value from getProperty.
// TODO: Remove once recordings no longer contain this field
const std::optional<TraceValue> value_;

GetPropertyRecord(
TimeSinceStart time,
ObjectID objID,
TraceValue propID,
TraceValue propID
#ifdef HERMESVM_API_TRACE_DEBUG
,
const std::string &propNameDbg
#endif
std::optional<TraceValue> value = std::nullopt)
)
: Record(time),
objID_(objID),
propID_(propID),
propID_(propID)
#ifdef HERMESVM_API_TRACE_DEBUG
propNameDbg_(propNameDbg),
,
propNameDbg_(propNameDbg)
#endif
value_(value) {
{
}

static constexpr RecordType type{RecordType::GetProperty};
Expand All @@ -778,14 +777,6 @@ class SynthTrace {
return uses;
}

std::vector<ObjectID> defs() const override {
std::vector<ObjectID> defs{};
if (value_) {
pushIfTrackedValue(*value_, defs);
}
return defs;
}

void toJSONInternal(::hermes::JSONEmitter &json) const override;
};

Expand Down Expand Up @@ -879,28 +870,14 @@ class SynthTrace {
static constexpr RecordType type{RecordType::GetPropertyNames};
/// The ObjectID of the object that was accessed for its property.
const ObjectID objID_;
// Since getPropertyNames always returns an array, this can be an object id
// rather than a TraceValue.
/// The ObjectID of the array that was returned by getPropertyNames().
// TODO: delete once no traces contain this field
const std::optional<ObjectID> propNamesID_;

explicit GetPropertyNamesRecord(
TimeSinceStart time,
ObjectID objID,
std::optional<ObjectID> propNamesID = std::nullopt)
: Record(time), objID_(objID), propNamesID_(propNamesID) {}
explicit GetPropertyNamesRecord(TimeSinceStart time, ObjectID objID)
: Record(time), objID_(objID) {}

void toJSONInternal(::hermes::JSONEmitter &json) const override;
RecordType getType() const override {
return type;
}
std::vector<ObjectID> defs() const override {
if (!propNamesID_) {
return {};
}
return {*propNamesID_};
}
std::vector<ObjectID> uses() const override {
return {objID_};
}
Expand Down Expand Up @@ -939,28 +916,14 @@ class SynthTrace {
const ObjectID objID_;
/// The index of the element that was accessed in the array.
const size_t index_;
/// The value that was read from the array.
// TODO: remove once recordings no longer contain this field
const std::optional<TraceValue> value_;

explicit ArrayReadRecord(
TimeSinceStart time,
ObjectID objID,
size_t index,
std::optional<TraceValue> value = std::nullopt)
: Record(time), objID_(objID), index_(index), value_(value) {}
explicit ArrayReadRecord(TimeSinceStart time, ObjectID objID, size_t index)
: Record(time), objID_(objID), index_(index) {}

static constexpr RecordType type{RecordType::ArrayRead};
RecordType getType() const override {
return type;
}
std::vector<ObjectID> defs() const override {
std::vector<ObjectID> defs{};
if (value_) {
pushIfTrackedValue(*value_, defs);
}
return defs;
}
std::vector<ObjectID> uses() const override {
return {objID_};
}
Expand Down
23 changes: 5 additions & 18 deletions API/hermes/SynthTraceParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -398,19 +398,15 @@ SynthTrace getTrace(
break;
}
case RecordType::GetProperty: {
std::optional<SynthTrace::TraceValue> value;
if (propValue) {
value = trace.decode(propValue->c_str());
}

trace.emplace_back<SynthTrace::GetPropertyRecord>(
timeFromStart,
objID->getValue(),
SynthTrace::decode(propID->str()),
SynthTrace::decode(propID->str())
#ifdef HERMESVM_API_TRACE_DEBUG
,
std::string(propName->c_str()),
#endif
value);
);
break;
}
case RecordType::SetProperty:
Expand All @@ -435,13 +431,8 @@ SynthTrace getTrace(
);
break;
case RecordType::GetPropertyNames: {
JSONValue *propNamesIDVal = obj->get("propNamesID");
std::optional<SynthTrace::ObjectID> propNamesId;
if (propNamesIDVal) {
propNamesId = getNumberAs<SynthTrace::ObjectID>(propNamesIDVal);
}
trace.emplace_back<SynthTrace::GetPropertyNamesRecord>(
timeFromStart, objID->getValue(), propNamesId);
timeFromStart, objID->getValue());
break;
}
case RecordType::CreateArray:
Expand All @@ -451,12 +442,8 @@ SynthTrace getTrace(
getNumberAs<uint64_t>(obj->get("length")));
break;
case RecordType::ArrayRead: {
std::optional<SynthTrace::TraceValue> value;
if (propValue) {
value = trace.decode(propValue->c_str());
}
trace.emplace_back<SynthTrace::ArrayReadRecord>(
timeFromStart, objID->getValue(), arrayIndex->getValue(), value);
timeFromStart, objID->getValue(), arrayIndex->getValue());
break;
}
case RecordType::ArrayWrite:
Expand Down
21 changes: 3 additions & 18 deletions API/hermes/TraceInterpreter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -855,12 +855,7 @@ void TraceInterpreter::executeRecords() {
#endif
value = obj.getProperty(rt_, propNameID);
}
if (gpr.value_) {
ifObjectAddToObjectMap(
*gpr.value_, std::move(value), currentExecIndex);
} else {
retval = std::move(value);
}
retval = std::move(value);
break;
}
case RecordType::SetProperty: {
Expand Down Expand Up @@ -912,12 +907,7 @@ void TraceInterpreter::executeRecords() {
jsi::Array arr = getJSIValueForUse(gpnr.objID_)
.asObject(rt_)
.getPropertyNames(rt_);
if (gpnr.propNamesID_) {
addToObjectMap(
*gpnr.propNamesID_, std::move(arr), currentExecIndex);
} else {
retval = std::move(arr);
}
retval = std::move(arr);
break;
}
case RecordType::CreateArray: {
Expand All @@ -935,12 +925,7 @@ void TraceInterpreter::executeRecords() {
.asObject(rt_)
.asArray(rt_)
.getValueAtIndex(rt_, arr.index_);
if (arr.value_) {
ifObjectAddToObjectMap(
*arr.value_, std::move(value), currentExecIndex);
} else {
retval = std::move(value);
}
retval = std::move(value);
break;
}
case RecordType::ArrayWrite: {
Expand Down

0 comments on commit 677377d

Please sign in to comment.