Avoid unnecessary copies of AST objects.

Use placement-new to copy AST data into fBytes; this gives us a "real
object" at that memory location which we can later access via
reinterpret_cast. In practical terms, this lets us return a reference
that points inside of fBytes without causing UB. This saves us from
needing to copy the data back out every time it is accessed.

For large objects, this can be a substantial savings. For sufficiently
small objects (<= 8 bytes) the data is copied back out regardless, since
a single load is more than fast enough.

Change-Id: I3ad246acda4feb7e7aa1987d966abd607983e205
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/380996
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This commit is contained in:
John Stiles 2021-03-08 11:15:55 -05:00
parent 9ff7eef906
commit ece1bf0afb
4 changed files with 58 additions and 82 deletions

View File

@ -87,7 +87,7 @@ String ASTNode::description() const {
(this->begin() + 1)->description() + "; " + (this->begin() + 2)->description() +
") " + (this->begin() + 3)->description();
case Kind::kFunction: {
FunctionData fd = getFunctionData();
const FunctionData& fd = getFunctionData();
String result = fd.fModifiers.description();
if (result.size()) {
result += " ";
@ -131,7 +131,7 @@ String ASTNode::description() const {
case Kind::kInt:
return to_string(getInt());
case Kind::kInterfaceBlock: {
InterfaceBlockData id = getInterfaceBlockData();
const InterfaceBlockData& id = getInterfaceBlockData();
String result = id.fModifiers.description() + " " + id.fTypeName + " {\n";
auto iter = this->begin();
for (size_t i = 0; i < id.fDeclarationCount; ++i) {
@ -149,7 +149,7 @@ String ASTNode::description() const {
case Kind::kModifiers:
return getModifiers().description();
case Kind::kParameter: {
ParameterData pd = getParameterData();
const ParameterData& pd = getParameterData();
auto iter = this->begin();
String result = (iter++)->description() + " " + pd.fName;
if (pd.fIsArray) {
@ -206,7 +206,7 @@ String ASTNode::description() const {
case Kind::kType:
return getString();
case Kind::kVarDeclaration: {
VarData vd = getVarData();
const VarData& vd = getVarData();
String result = vd.fName;
auto iter = this->begin();
if (vd.fIsArray) {
@ -238,7 +238,7 @@ String ASTNode::description() const {
}
default:
SkASSERT(false);
SkDEBUGFAIL("unrecognized AST node kind");
return "<error>";
}
}

View File

@ -251,7 +251,10 @@ struct ASTNode {
};
struct NodeData {
char fBytes[std::max({sizeof(Token::Kind),
// We use fBytes as a union which can hold any type of AST node, and use placement-new to
// copy AST objects into fBytes. Note that none of the AST objects have interesting
// destructors, so we do not bother doing a placement-delete on any of them in ~NodeData.
char fBytes[std::max({sizeof(Operator),
sizeof(StringFragment),
sizeof(bool),
sizeof(SKSL_INT),
@ -281,58 +284,57 @@ struct ASTNode {
NodeData(Operator op)
: fKind(Kind::kOperator) {
Token::Kind data = op.kind();
memcpy(fBytes, &data, sizeof(data));
new (fBytes) Operator(op);
}
NodeData(StringFragment data)
NodeData(const StringFragment& data)
: fKind(Kind::kStringFragment) {
memcpy(fBytes, &data, sizeof(data));
new (fBytes) StringFragment(data);
}
NodeData(bool data)
: fKind(Kind::kBool) {
memcpy(fBytes, &data, sizeof(data));
new (fBytes) bool(data);
}
NodeData(SKSL_INT data)
: fKind(Kind::kInt) {
memcpy(fBytes, &data, sizeof(data));
new (fBytes) SKSL_INT(data);
}
NodeData(SKSL_FLOAT data)
: fKind(Kind::kFloat) {
memcpy(fBytes, &data, sizeof(data));
new (fBytes) SKSL_FLOAT(data);
}
NodeData(Modifiers data)
NodeData(const Modifiers& data)
: fKind(Kind::kModifiers) {
memcpy(fBytes, &data, sizeof(data));
new (fBytes) Modifiers(data);
}
NodeData(FunctionData data)
NodeData(const FunctionData& data)
: fKind(Kind::kFunctionData) {
memcpy(fBytes, &data, sizeof(data));
new (fBytes) FunctionData(data);
}
NodeData(VarData data)
NodeData(const VarData& data)
: fKind(Kind::kVarData) {
memcpy(fBytes, &data, sizeof(data));
new (fBytes) VarData(data);
}
NodeData(ParameterData data)
NodeData(const ParameterData& data)
: fKind(Kind::kParameterData) {
memcpy(fBytes, &data, sizeof(data));
new (fBytes) ParameterData(data);
}
NodeData(InterfaceBlockData data)
NodeData(const InterfaceBlockData& data)
: fKind(Kind::kInterfaceBlockData) {
memcpy(fBytes, &data, sizeof(data));
new (fBytes) InterfaceBlockData(data);
}
NodeData(SectionData data)
NodeData(const SectionData& data)
: fKind(Kind::kSectionData) {
memcpy(fBytes, &data, sizeof(data));
new (fBytes) SectionData(data);
}
};
@ -455,103 +457,77 @@ struct ASTNode {
Operator getOperator() const {
SkASSERT(fData.fKind == NodeData::Kind::kOperator);
Token::Kind tokenKind;
memcpy(&tokenKind, fData.fBytes, sizeof(tokenKind));
return Operator{tokenKind};
return *reinterpret_cast<const Operator*>(fData.fBytes);
}
bool getBool() const {
SkASSERT(fData.fKind == NodeData::Kind::kBool);
bool result;
memcpy(&result, fData.fBytes, sizeof(result));
return result;
return *reinterpret_cast<const bool*>(fData.fBytes);
}
SKSL_INT getInt() const {
SkASSERT(fData.fKind == NodeData::Kind::kInt);
SKSL_INT result;
memcpy(&result, fData.fBytes, sizeof(result));
return result;
return *reinterpret_cast<const SKSL_INT*>(fData.fBytes);
}
SKSL_FLOAT getFloat() const {
SkASSERT(fData.fKind == NodeData::Kind::kFloat);
SKSL_FLOAT result;
memcpy(&result, fData.fBytes, sizeof(result));
return result;
return *reinterpret_cast<const SKSL_FLOAT*>(fData.fBytes);
}
StringFragment getString() const {
const StringFragment& getString() const {
SkASSERT(fData.fKind == NodeData::Kind::kStringFragment);
StringFragment result;
memcpy(&result, fData.fBytes, sizeof(result));
return result;
return *reinterpret_cast<const StringFragment*>(fData.fBytes);
}
Modifiers getModifiers() const {
const Modifiers& getModifiers() const {
SkASSERT(fData.fKind == NodeData::Kind::kModifiers);
Modifiers result;
memcpy(&result, fData.fBytes, sizeof(result));
return result;
return *reinterpret_cast<const Modifiers*>(fData.fBytes);
}
void setModifiers(const Modifiers& m) {
memcpy(fData.fBytes, &m, sizeof(m));
new (fData.fBytes) Modifiers(m);
}
ParameterData getParameterData() const {
const ParameterData& getParameterData() const {
SkASSERT(fData.fKind == NodeData::Kind::kParameterData);
ParameterData result;
memcpy(&result, fData.fBytes, sizeof(result));
return result;
return *reinterpret_cast<const ParameterData*>(fData.fBytes);
}
void setParameterData(const ASTNode::ParameterData& pd) {
SkASSERT(fData.fKind == NodeData::Kind::kParameterData);
memcpy(fData.fBytes, &pd, sizeof(pd));
new (fData.fBytes) ParameterData(pd);
}
VarData getVarData() const {
const VarData& getVarData() const {
SkASSERT(fData.fKind == NodeData::Kind::kVarData);
VarData result;
memcpy(&result, fData.fBytes, sizeof(result));
return result;
return *reinterpret_cast<const VarData*>(fData.fBytes);
}
void setVarData(const ASTNode::VarData& vd) {
SkASSERT(fData.fKind == NodeData::Kind::kVarData);
memcpy(fData.fBytes, &vd, sizeof(vd));
new (fData.fBytes) VarData(vd);
}
FunctionData getFunctionData() const {
const FunctionData& getFunctionData() const {
SkASSERT(fData.fKind == NodeData::Kind::kFunctionData);
FunctionData result;
memcpy(&result, fData.fBytes, sizeof(result));
return result;
return *reinterpret_cast<const FunctionData*>(fData.fBytes);
}
void setFunctionData(const ASTNode::FunctionData& fd) {
SkASSERT(fData.fKind == NodeData::Kind::kFunctionData);
memcpy(fData.fBytes, &fd, sizeof(fd));
new (fData.fBytes) FunctionData(fd);
}
InterfaceBlockData getInterfaceBlockData() const {
const InterfaceBlockData& getInterfaceBlockData() const {
SkASSERT(fData.fKind == NodeData::Kind::kInterfaceBlockData);
InterfaceBlockData result;
memcpy(&result, fData.fBytes, sizeof(result));
return result;
return *reinterpret_cast<const InterfaceBlockData*>(fData.fBytes);
}
void setInterfaceBlockData(const ASTNode::InterfaceBlockData& id) {
SkASSERT(fData.fKind == NodeData::Kind::kInterfaceBlockData);
memcpy(fData.fBytes, &id, sizeof(id));
new (fData.fBytes) InterfaceBlockData(id);
}
SectionData getSectionData() const {
const SectionData& getSectionData() const {
SkASSERT(fData.fKind == NodeData::Kind::kSectionData);
SectionData result;
memcpy(&result, fData.fBytes, sizeof(result));
return result;
return *reinterpret_cast<const SectionData*>(fData.fBytes);
}
void addChild(ID id) {

View File

@ -1057,7 +1057,7 @@ void IRGenerator::convertFunction(const ASTNode& f) {
for (size_t i = 0; i < funcData.fParameterCount; ++i) {
const ASTNode& param = *(iter++);
SkASSERT(param.fKind == ASTNode::Kind::kParameter);
ASTNode::ParameterData pd = param.getParameterData();
const ASTNode::ParameterData& pd = param.getParameterData();
this->checkModifiers(param.fOffset, pd.fModifiers,
Modifiers::kConst_Flag | Modifiers::kIn_Flag | Modifiers::kOut_Flag,
/*permittedLayoutFlags=*/0);
@ -1281,7 +1281,7 @@ std::unique_ptr<InterfaceBlock> IRGenerator::convertInterfaceBlock(const ASTNode
}
SkASSERT(intf.fKind == ASTNode::Kind::kInterfaceBlock);
ASTNode::InterfaceBlockData id = intf.getInterfaceBlockData();
const ASTNode::InterfaceBlockData& id = intf.getInterfaceBlockData();
std::shared_ptr<SymbolTable> old = fSymbolTable;
std::shared_ptr<SymbolTable> symbols;
std::vector<Type::Field> fields;
@ -1554,7 +1554,7 @@ std::unique_ptr<Expression> IRGenerator::convertIdentifier(int offset, StringFra
bool valid = false;
for (const auto& decl : fFile->root()) {
if (decl.fKind == ASTNode::Kind::kSection) {
ASTNode::SectionData section = decl.getSectionData();
const ASTNode::SectionData& section = decl.getSectionData();
if (section.fName == "setData") {
valid = true;
break;
@ -1604,7 +1604,7 @@ std::unique_ptr<Section> IRGenerator::convertSection(const ASTNode& s) {
return nullptr;
}
ASTNode::SectionData section = s.getSectionData();
const ASTNode::SectionData& section = s.getSectionData();
return std::make_unique<Section>(s.fOffset, section.fName, section.fArgument,
section.fText);
}
@ -2040,7 +2040,7 @@ std::unique_ptr<Expression> IRGenerator::convertFieldExpression(const ASTNode& f
if (!base) {
return nullptr;
}
StringFragment field = fieldNode.getString();
const StringFragment& field = fieldNode.getString();
const Type& baseType = base->type();
if (baseType == *fContext.fTypes.fSkCaps) {
return Setting::Convert(fContext, fieldNode.fOffset, field);
@ -2062,7 +2062,7 @@ std::unique_ptr<Expression> IRGenerator::convertScopeExpression(const ASTNode& s
this->errorReporter().error(scopeNode.fOffset, "'::' must follow a type name");
return nullptr;
}
StringFragment member = scopeNode.getString();
const StringFragment& member = scopeNode.getString();
return this->convertTypeField(base->fOffset, base->as<TypeReference>().value(), member);
}

View File

@ -569,7 +569,7 @@ ASTNode::ID Parser::structDeclaration() {
return ASTNode::ID::Invalid();
}
ASTNode& declsNode = getNode(decls);
Modifiers modifiers = declsNode.begin()->getModifiers();
const Modifiers& modifiers = declsNode.begin()->getModifiers();
if (modifiers.fFlags != Modifiers::kNo_Flag) {
String desc = modifiers.description();
desc.pop_back(); // remove trailing space
@ -587,7 +587,7 @@ ASTNode::ID Parser::structDeclaration() {
for (auto iter = declsNode.begin() + 2; iter != declsNode.end(); ++iter) {
ASTNode& var = *iter;
ASTNode::VarData vd = var.getVarData();
const ASTNode::VarData& vd = var.getVarData();
// Read array size if one is present.
if (vd.fIsArray) {