Skip to content

Commit

Permalink
[NFC] Make the GCData constructor a move constructor (#6946)
Browse files Browse the repository at this point in the history
This avoids creating a large Literals (SmallVector of Literal) and then copying it. All the places
that construct GCData do not need the Literals afterwards.

This gives a 7% speedup on the --precompute benchmark from #6931
  • Loading branch information
kripken authored Sep 17, 2024
1 parent bdc7ce0 commit a99b48a
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 12 deletions.
3 changes: 2 additions & 1 deletion src/literal.h
Original file line number Diff line number Diff line change
Expand Up @@ -763,7 +763,8 @@ struct GCData {
// The element or field values.
Literals values;

GCData(HeapType type, Literals values) : type(type), values(values) {}
GCData(HeapType type, Literals&& values)
: type(type), values(std::move(values)) {}
};

// The data of a (ref exn) literal.
Expand Down
20 changes: 20 additions & 0 deletions src/support/small_vector.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,13 +65,33 @@ template<typename T, size_t N> class SmallVector {
using value_type = T;

SmallVector() {}
SmallVector(const SmallVector<T, N>& other)
: usedFixed(other.usedFixed), fixed(other.fixed), flexible(other.flexible) {
}
SmallVector(SmallVector<T, N>&& other)
: usedFixed(other.usedFixed), fixed(std::move(other.fixed)),
flexible(std::move(other.flexible)) {}
SmallVector(std::initializer_list<T> init) {
for (T item : init) {
push_back(item);
}
}
SmallVector(size_t initialSize) { resize(initialSize); }

SmallVector<T, N>& operator=(const SmallVector<T, N>& other) {
usedFixed = other.usedFixed;
fixed = other.fixed;
flexible = other.flexible;
return *this;
}

SmallVector<T, N>& operator=(SmallVector<T, N>&& other) {
usedFixed = other.usedFixed;
fixed = std::move(other.fixed);
flexible = std::move(other.flexible);
return *this;
}

T& operator[](size_t i) {
if (i < N) {
return fixed[i];
Expand Down
23 changes: 13 additions & 10 deletions src/wasm-interpreter.h
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,11 @@ class ExpressionRunner : public OverriddenVisitor<SubType, Flow> {
// data, as we don't have a cycle collector. Those leaks are not a serious
// problem as Binaryen is not really used in long-running tasks, so we ignore
// this function in LSan.
Literal makeGCData(const Literals& data, Type type) {
auto allocation = std::make_shared<GCData>(type.getHeapType(), data);
//
// This consumes the input |data| entirely.
Literal makeGCData(Literals&& data, Type type) {
auto allocation =
std::make_shared<GCData>(type.getHeapType(), std::move(data));
#if __has_feature(leak_sanitizer) || __has_feature(address_sanitizer)
// GC data with cycles will leak, since shared_ptrs do not handle cycles.
// Binaryen is generally not used in long-running programs so we just ignore
Expand Down Expand Up @@ -1655,7 +1658,7 @@ class ExpressionRunner : public OverriddenVisitor<SubType, Flow> {
data[i] = truncateForPacking(value.getSingleValue(), field);
}
}
return makeGCData(data, curr->type);
return makeGCData(std::move(data), curr->type);
}
Flow visitStructGet(StructGet* curr) {
NOTE_ENTER("StructGet");
Expand Down Expand Up @@ -1735,7 +1738,7 @@ class ExpressionRunner : public OverriddenVisitor<SubType, Flow> {
data[i] = value;
}
}
return makeGCData(data, curr->type);
return makeGCData(std::move(data), curr->type);
}
Flow visitArrayNewData(ArrayNewData* curr) { WASM_UNREACHABLE("unimp"); }
Flow visitArrayNewElem(ArrayNewElem* curr) { WASM_UNREACHABLE("unimp"); }
Expand Down Expand Up @@ -1766,7 +1769,7 @@ class ExpressionRunner : public OverriddenVisitor<SubType, Flow> {
}
data[i] = truncateForPacking(value.getSingleValue(), field);
}
return makeGCData(data, curr->type);
return makeGCData(std::move(data), curr->type);
}
Flow visitArrayGet(ArrayGet* curr) {
NOTE_ENTER("ArrayGet");
Expand Down Expand Up @@ -1971,7 +1974,7 @@ class ExpressionRunner : public OverriddenVisitor<SubType, Flow> {
contents.push_back(ptrDataValues[i]);
}
}
return makeGCData(contents, curr->type);
return makeGCData(std::move(contents), curr->type);
}
case StringNewFromCodePoint: {
uint32_t codePoint = ptr.getSingleValue().getUnsigned();
Expand Down Expand Up @@ -2041,7 +2044,7 @@ class ExpressionRunner : public OverriddenVisitor<SubType, Flow> {
contents.push_back(l);
}

return makeGCData(contents, curr->type);
return makeGCData(std::move(contents), curr->type);
}
Flow visitStringEncode(StringEncode* curr) {
// For now we only support JS-style strings into arrays.
Expand Down Expand Up @@ -2203,7 +2206,7 @@ class ExpressionRunner : public OverriddenVisitor<SubType, Flow> {
}
}
}
return makeGCData(contents, curr->type);
return makeGCData(std::move(contents), curr->type);
}

virtual void trap(const char* why) { WASM_UNREACHABLE("unimp"); }
Expand Down Expand Up @@ -4010,7 +4013,7 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
auto addr = (void*)&seg.data[i];
contents.push_back(this->makeFromMemory(addr, element));
}
return self()->makeGCData(contents, curr->type);
return self()->makeGCData(std::move(contents), curr->type);
}
Flow visitArrayNewElem(ArrayNewElem* curr) {
NOTE_ENTER("ArrayNewElem");
Expand Down Expand Up @@ -4041,7 +4044,7 @@ class ModuleRunnerBase : public ExpressionRunner<SubType> {
auto val = self()->visit(seg.data[i]).getSingleValue();
contents.push_back(val);
}
return self()->makeGCData(contents, curr->type);
return self()->makeGCData(std::move(contents), curr->type);
}
Flow visitArrayInitData(ArrayInitData* curr) {
NOTE_ENTER("ArrayInit");
Expand Down
2 changes: 1 addition & 1 deletion src/wasm/literal.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ Literal::Literal(std::string_view string)
int32_t u = uint8_t(string[i]) | (uint8_t(string[i + 1]) << 8);
contents.push_back(Literal(u));
}
gcData = std::make_shared<GCData>(HeapType::string, contents);
gcData = std::make_shared<GCData>(HeapType::string, std::move(contents));
}

Literal::Literal(const Literal& other) : type(other.type) {
Expand Down

0 comments on commit a99b48a

Please sign in to comment.