AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Simplify the ModifiersPool class.

Browse Source 
Rather than adding unique Modifiers to a vector and then returning
vector indices as opaque Handle objects, we can instead add them to an
unordered_set and return back the address of the object inside the set.
This removes a lot of complexity and saves an indirection.

STL unordered_sets guarantee pointer stability, so this is safe:
https://en.cppreference.com/w/cpp/container/unordered_set/insert
"If rehashing occurs due to the insertion, all iterators are
invalidated. Otherwise iterators are not affected. References are not
invalidated."

Change-Id: I580cad12b3711d490baab417affb8895f7fa18e7
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/334598
Auto-Submit: John Stiles <johnstiles@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This commit is contained in:
John Stiles 2020-11-12 18:27:13 -05:00 committed by Skia Commit-Bot
parent 011218edb5
commit 586df9556c
9 changed files with 48 additions and 92 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/sksl/SkSLCompiler.cpp
View File

@ -167,7 +167,7 @@ Compiler::Compiler(const ShaderCapsClass* caps, Flags flags)
// treat it as builtin (ie, no need to clone it into the Program).
fPrivateSymbolTable->add(
std::make_unique<Variable>(/*offset=*/-1,
fIRGenerator->fModifiers->handle(Modifiers()),
fIRGenerator->fModifiers->addToPool(Modifiers()),
"sk_Caps",
fContext->fSkCaps_Type.get(),
/*builtin=*/false,

45
src/sksl/SkSLIRGenerator.cpp
View File

@ -415,7 +415,7 @@ StatementArray IRGenerator::convertVarDeclarations(const ASTNode& decls,
sizes.push_back(nullptr);
}
}
auto var = std::make_unique<Variable>(varDecl.fOffset, fModifiers->handle(modifiers),
auto var = std::make_unique<Variable>(varDecl.fOffset, fModifiers->addToPool(modifiers),
varData.fName, type, fIsBuiltinCode, storage);
if (var->name() == Compiler::RTADJUST_NAME) {
SkASSERT(!fRTAdjust);
@ -476,7 +476,7 @@ std::unique_ptr<ModifiersDeclaration> IRGenerator::convertModifiersDeclaration(c
!fCaps->gsInvocationsSupport()) {
modifiers.fLayout.fMaxVertices *= fInvocations;
}
return std::make_unique<ModifiersDeclaration>(fModifiers->handle(modifiers));
return std::make_unique<ModifiersDeclaration>(fModifiers->addToPool(modifiers));
}
std::unique_ptr<Statement> IRGenerator::convertIf(const ASTNode& n) {
@ -727,14 +727,13 @@ std::unique_ptr<Statement> IRGenerator::convertDiscard(const ASTNode& d) {
std::unique_ptr<Block> IRGenerator::applyInvocationIDWorkaround(std::unique_ptr<Block> main) {
Layout invokeLayout;
Modifiers invokeModifiers(invokeLayout, Modifiers::kHasSideEffects_Flag);
const FunctionDeclaration* invokeDecl =
fSymbolTable->add(std::make_unique<FunctionDeclaration>(
/*offset=*/-1,
fModifiers->handle(invokeModifiers),
"_invoke",
std::vector<const Variable*>(),
fContext.fVoid_Type.get(),
fIsBuiltinCode));
const FunctionDeclaration* invokeDecl = fSymbolTable->add(std::make_unique<FunctionDeclaration>(
/*offset=*/-1,
fModifiers->addToPool(invokeModifiers),
"_invoke",
std::vector<const Variable*>(),
fContext.fVoid_Type.get(),
fIsBuiltinCode));
fProgramElements->push_back(std::make_unique<FunctionDefinition>(/*offset=*/-1,
invokeDecl, fIsBuiltinCode,
std::move(main)));
@ -950,7 +949,7 @@ void IRGenerator::convertFunction(const ASTNode& f) {
}
const Variable* var = fSymbolTable->takeOwnershipOfSymbol(
std::make_unique<Variable>(param.fOffset, fModifiers->handle(m), pd.fName, type,
std::make_unique<Variable>(param.fOffset, fModifiers->addToPool(m), pd.fName, type,
fIsBuiltinCode, Variable::Storage::kParameter));
parameters.push_back(var);
}
@ -1022,7 +1021,7 @@ void IRGenerator::convertFunction(const ASTNode& f) {
if (match) {
if (*returnType != other->returnType()) {
FunctionDeclaration newDecl(f.fOffset,
fModifiers->handle(funcData.fModifiers),
fModifiers->addToPool(funcData.fModifiers),
funcData.fName,
parameters,
returnType,
@ -1057,13 +1056,13 @@ void IRGenerator::convertFunction(const ASTNode& f) {
}
// Create a new declaration.
decl = fSymbolTable->add(std::make_unique<FunctionDeclaration>(
f.fOffset,
fModifiers->handle(declModifiers),
funcData.fName,
parameters,
returnType,
fIsBuiltinCode));
decl = fSymbolTable->add(
std::make_unique<FunctionDeclaration>(f.fOffset,
fModifiers->addToPool(declModifiers),
funcData.fName,
parameters,
returnType,
fIsBuiltinCode));
}
if (iter == f.end()) {
// If there's no body, we've found a prototype.
@ -1185,7 +1184,7 @@ std::unique_ptr<InterfaceBlock> IRGenerator::convertInterfaceBlock(const ASTNode
}
const Variable* var = old->takeOwnershipOfSymbol(
std::make_unique<Variable>(intf.fOffset,
fModifiers->handle(id.fModifiers),
fModifiers->addToPool(id.fModifiers),
id.fInstanceName.fLength ? id.fInstanceName : id.fTypeName,
type,
fIsBuiltinCode,
@ -1256,7 +1255,7 @@ void IRGenerator::convertEnum(const ASTNode& e) {
}
value = std::unique_ptr<Expression>(new IntLiteral(fContext, e.fOffset, currentValue));
++currentValue;
fSymbolTable->add(std::make_unique<Variable>(e.fOffset, fModifiers->handle(modifiers),
fSymbolTable->add(std::make_unique<Variable>(e.fOffset, fModifiers->addToPool(modifiers),
child.getString(), type, fIsBuiltinCode,
Variable::Storage::kGlobal, value.get()));
fSymbolTable->takeOwnershipOfIRNode(std::move(value));
@ -2894,7 +2893,7 @@ void IRGenerator::cloneBuiltinVariables() {
// so we're pointing at a Program-owned expression.
const Variable* clonedVar =
fGenerator->fSymbolTable->takeOwnershipOfSymbol(std::make_unique<Variable>(
sharedVar->fOffset, sharedVar->modifiersHandle(), sharedVar->name(),
sharedVar->fOffset, &sharedVar->modifiers(), sharedVar->name(),
&sharedVar->type(), /*builtin=*/false, sharedVar->storage(),
initialValue));
@ -3005,7 +3004,7 @@ IRGenerator::IRBundle IRGenerator::convertProgram(
m.fFlags = Modifiers::kIn_Flag;
m.fLayout.fBuiltin = SK_INVOCATIONID_BUILTIN;
}
auto var = std::make_unique<Variable>(-1, fModifiers->handle(m), "sk_InvocationID",
auto var = std::make_unique<Variable>(-1, fModifiers->addToPool(m), "sk_InvocationID",
fContext.fInt_Type.get(), false,
Variable::Storage::kGlobal);
auto decl = std::make_unique<VarDeclaration>(var.get(), fContext.fInt_Type.get(),

4
src/sksl/SkSLInliner.cpp
View File

@ -564,7 +564,7 @@ std::unique_ptr<Statement> Inliner::inlineStatement(int offset,
const Type* typePtr = copy_if_needed(&old.type(), *symbolTableForStatement);
const Variable* clone = symbolTableForStatement->takeOwnershipOfSymbol(
std::make_unique<Variable>(offset,
old.modifiersHandle(),
&old.modifiers(),
namePtr->c_str(),
typePtr,
isBuiltinCode,
@ -644,7 +644,7 @@ Inliner::InlinedCall Inliner::inlineCall(FunctionCall* call,
// Add our new variable to the symbol table.
const Variable* variableSymbol = symbolTableForCall->add(std::make_unique<Variable>(
/*offset=*/-1, fModifiers->handle(Modifiers()),
/*offset=*/-1, fModifiers->addToPool(Modifiers()),
nameFrag, type, caller->isBuiltin(),
Variable::Storage::kLocal, initialValue->get()));

49
src/sksl/SkSLModifiersPool.h
View File

@ -8,60 +8,25 @@
#ifndef SKSL_MODIFIERSPOOL
#define SKSL_MODIFIERSPOOL
#include <unordered_map>
#include <unordered_set>
namespace SkSL {
struct Modifiers;
/**
* Stores a pool of Modifiers objects. Modifiers is fairly heavy, so to reduce IRNode's size we only
* store a handle to the Modifiers inside of the node and keep the object itself in a ModifiersPool.
* Deduplicates Modifiers objects and stores them in a shared pool. Modifiers are fairly heavy, and
* tend to be reused a lot, so deduplication can be a significant win.
*/
class ModifiersPool {
public:
class Handle {
public:
Handle() = default;
Handle(const ModifiersPool* pool, int index)
: fPool(pool)
, fIndex(index) {}
const Modifiers* operator->() const {
return &fPool->fModifiers[fIndex];
}
const Modifiers& operator*() const {
return fPool->fModifiers[fIndex];
}
private:
const ModifiersPool* fPool;
int fIndex;
};
bool empty() {
return fModifiers.empty();
}
Handle handle(const Modifiers& modifiers) {
SkASSERT(fModifiers.size() == fModifiersMap.size());
int index;
auto found = fModifiersMap.find(modifiers);
if (found != fModifiersMap.end()) {
index = found->second;
} else {
index = fModifiers.size();
fModifiers.push_back(modifiers);
fModifiersMap.insert({modifiers, index});
}
return Handle(this, index);
const Modifiers* addToPool(const Modifiers& modifiers) {
auto [iter, wasInserted] = fModifiersSet.insert(modifiers);
return &*iter;
}
private:
std::vector<Modifiers> fModifiers;
std::unordered_map<Modifiers, int> fModifiersMap;
std::unordered_set<Modifiers> fModifiersSet;
};
} // namespace SkSL

4
src/sksl/SkSLRehydrator.cpp
View File

@ -181,7 +181,7 @@ const Symbol* Rehydrator::symbol() {
const Type* returnType = this->type();
const FunctionDeclaration* result =
fSymbolTable->takeOwnershipOfSymbol(std::make_unique<FunctionDeclaration>(
/*offset=*/-1, fModifiers.handle(modifiers), name,
/*offset=*/-1, fModifiers.addToPool(modifiers), name,
std::move(parameters), returnType, /*builtin=*/true));
this->addSymbol(id, result);
return result;
@ -257,7 +257,7 @@ const Symbol* Rehydrator::symbol() {
}
case kVariable_Command: {
uint16_t id = this->readU16();
ModifiersPool::Handle m = fModifiers.handle(this->modifiers());
const Modifiers* m = fModifiers.addToPool(this->modifiers());
StringFragment name = this->readString();
const Type* type = this->type();
Variable::Storage storage = (Variable::Storage) this->readU8();

2
src/sksl/SkSLSPIRVCodeGenerator.cpp
View File

@ -1884,7 +1884,7 @@ SpvId SPIRVCodeGenerator::writeVariableReference(const VariableReference& ref, O
Modifiers modifiers(layout, Modifiers::kUniform_Flag);
const Variable* intfVar = fSynthetics.takeOwnershipOfSymbol(
std::make_unique<Variable>(/*offset=*/-1,
fProgram.fModifiers->handle(modifiers),
fProgram.fModifiers->addToPool(modifiers),
name,
&intfStruct,
/*builtin=*/false,

8
src/sksl/ir/SkSLFunctionDeclaration.h
View File

@ -27,18 +27,18 @@ class FunctionDeclaration final : public Symbol {
public:
static constexpr Kind kSymbolKind = Kind::kFunctionDeclaration;
FunctionDeclaration(int offset, ModifiersPool::Handle modifiers, StringFragment name,
FunctionDeclaration(int offset, const Modifiers* modifiers, StringFragment name,
std::vector<const Variable*> parameters, const Type* returnType,
bool builtin)
: INHERITED(offset, kSymbolKind, name, /*type=*/nullptr)
, fDefinition(nullptr)
, fModifiersHandle(modifiers)
, fModifiers(modifiers)
, fParameters(std::move(parameters))
, fReturnType(returnType)
, fBuiltin(builtin) {}
const Modifiers& modifiers() const {
return *fModifiersHandle;
return *fModifiers;
}
const FunctionDefinition* definition() const {
@ -147,7 +147,7 @@ public:
private:
mutable const FunctionDefinition* fDefinition;
ModifiersPool::Handle fModifiersHandle;
const Modifiers* fModifiers;
std::vector<const Variable*> fParameters;
const Type* fReturnType;
bool fBuiltin;

14
src/sksl/ir/SkSLModifiersDeclaration.h
View File

@ -22,20 +22,16 @@ class ModifiersDeclaration final : public ProgramElement {
public:
static constexpr Kind kProgramElementKind = Kind::kModifiers;
ModifiersDeclaration(ModifiersPool::Handle modifiers)
ModifiersDeclaration(const Modifiers* modifiers)
: INHERITED(-1, kProgramElementKind)
, fModifiersHandle(modifiers) {}
, fModifiers(modifiers) {}
const Modifiers& modifiers() const {
return *fModifiersHandle;
}
const ModifiersPool::Handle& modifiersHandle() const {
return fModifiersHandle;
return *fModifiers;
}
std::unique_ptr<ProgramElement> clone() const override {
return std::unique_ptr<ProgramElement>(new ModifiersDeclaration(this->modifiersHandle()));
return std::make_unique<ModifiersDeclaration>(&this->modifiers());
}
String description() const override {
@ -43,7 +39,7 @@ public:
}
private:
ModifiersPool::Handle fModifiersHandle;
const Modifiers* fModifiers;
using INHERITED = ProgramElement;
};

12
src/sksl/ir/SkSLVariable.h
View File

@ -36,20 +36,16 @@ public:
static constexpr Kind kSymbolKind = Kind::kVariable;
Variable(int offset, ModifiersPool::Handle modifiers, StringFragment name, const Type* type,
Variable(int offset, const Modifiers* modifiers, StringFragment name, const Type* type,
bool builtin, Storage storage, const Expression* initialValue = nullptr)
: INHERITED(offset, kSymbolKind, name, type)
, fInitialValue(initialValue)
, fModifiersHandle(modifiers)
, fModifiers(modifiers)
, fStorage(storage)
, fBuiltin(builtin) {}
const Modifiers& modifiers() const {
return *fModifiersHandle;
}
const ModifiersPool::Handle& modifiersHandle() const {
return fModifiersHandle;
return *fModifiers;
}
bool isBuiltin() const {
@ -75,7 +71,7 @@ public:
private:
const Expression* fInitialValue = nullptr;
ModifiersPool::Handle fModifiersHandle;
const Modifiers* fModifiers;
VariableStorage fStorage;
bool fBuiltin;