Improve performance of SymbolTable lookups.

This CL introduces two significant changes:
- unordered_map is replaced by SkTHashMap.
- The StringFragment key is replaced by a SymbolKey class, which caches
  the hash of the string.

The second change is responsible for the lion's share of the measured
performance improvement. SymbolTable::operator[] was previously re-
hashing the string every time it recursed into the parent symbol table.

Nanobench shows a fairly consistent ~8% speed improvement in both
sksl_medium and sksl_large: http://screen/3KTc27gt85HS4wK

Change-Id: Ia1010dcd40d7f7ecdce1aa5bb10c6cf6f6f11a80
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/324628
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
Commit-Queue: Brian Osman <brianosman@google.com>
Commit-Queue: John Stiles <johnstiles@google.com>
This commit is contained in:
John Stiles 2020-10-09 12:48:04 -04:00 committed by Skia Commit-Bot
parent 118706a806
commit efe767da91
5 changed files with 56 additions and 36 deletions

View File

@ -242,11 +242,11 @@ void Dehydrator::write(const SymbolTable& symbols) {
for (const std::unique_ptr<const Symbol>& s : symbols.fOwnedSymbols) {
this->write(*s);
}
this->writeU16(symbols.fSymbols.size());
this->writeU16(symbols.fSymbols.count());
std::map<StringFragment, const Symbol*> ordered;
for (std::pair<StringFragment, const Symbol*> p : symbols.fSymbols) {
ordered.insert(p);
}
symbols.foreach([&](StringFragment name, const Symbol* symbol) {
ordered.insert({name, symbol});
});
for (std::pair<StringFragment, const Symbol*> p : ordered) {
bool found = false;
for (size_t i = 0; i < symbols.fOwnedSymbols.size(); ++i) {

View File

@ -35,9 +35,10 @@ struct StringFragment {
: fChars(chars)
, fLength(length) {}
char operator[](size_t idx) const {
return fChars[idx];
}
const char* data() const { return fChars; }
size_t size() const { return fLength; }
size_t length() const { return fLength; }
char operator[](size_t idx) const { return fChars[idx]; }
bool operator==(const char* s) const;
bool operator!=(const char* s) const;

View File

@ -49,9 +49,10 @@ public:
String result = "enum class " + this->typeName() + " {\n";
String separator;
std::vector<const Symbol*> sortedSymbols;
for (const auto& pair : *this->symbols()) {
sortedSymbols.push_back(pair.second);
}
sortedSymbols.reserve(symbols()->count());
this->symbols()->foreach([&](StringFragment, const Symbol* symbol) {
sortedSymbols.push_back(symbol);
});
std::sort(sortedSymbols.begin(), sortedSymbols.end(),
[](const Symbol* a, const Symbol* b) { return a->name() < b->name(); });
for (const auto& s : sortedSymbols) {

View File

@ -24,18 +24,24 @@ std::vector<const FunctionDeclaration*> SymbolTable::GetFunctions(const Symbol&
}
Symbol* SymbolTable::operator[](StringFragment name) {
auto entry = fSymbols.find(name);
if (entry == fSymbols.end()) {
return this->lookup(MakeSymbolKey(name));
}
Symbol* SymbolTable::lookup(const SymbolKey& key) {
Symbol** symbolPPtr = fSymbols.find(key);
if (!symbolPPtr) {
if (fParent) {
return (*fParent)[name];
return fParent->lookup(key);
}
return nullptr;
}
Symbol* symbol = *symbolPPtr;
if (fParent) {
auto functions = GetFunctions(*entry->second);
auto functions = GetFunctions(*symbol);
if (functions.size() > 0) {
bool modified = false;
const Symbol* previous = (*fParent)[name];
const Symbol* previous = fParent->lookup(key);
if (previous) {
auto previousFunctions = GetFunctions(*previous);
for (const FunctionDeclaration* prev : previousFunctions) {
@ -59,7 +65,6 @@ Symbol* SymbolTable::operator[](StringFragment name) {
}
}
}
Symbol* symbol = entry->second;
while (symbol && symbol->is<SymbolAlias>()) {
symbol = symbol->as<SymbolAlias>().origSymbol();
}
@ -79,7 +84,7 @@ void SymbolTable::addAlias(StringFragment name, Symbol* symbol) {
void SymbolTable::addWithoutOwnership(Symbol* symbol) {
const StringFragment& name = symbol->name();
Symbol*& refInSymbolTable = fSymbols[name];
Symbol*& refInSymbolTable = fSymbols[MakeSymbolKey(name)];
if (refInSymbolTable == nullptr) {
refInSymbolTable = symbol;
return;
@ -106,12 +111,4 @@ void SymbolTable::addWithoutOwnership(Symbol* symbol) {
}
}
std::unordered_map<StringFragment, Symbol*>::iterator SymbolTable::begin() {
return fSymbols.begin();
}
std::unordered_map<StringFragment, Symbol*>::iterator SymbolTable::end() {
return fSymbols.end();
}
} // namespace SkSL

View File

@ -8,12 +8,13 @@
#ifndef SKSL_SYMBOLTABLE
#define SKSL_SYMBOLTABLE
#include <unordered_map>
#include <memory>
#include <vector>
#include "include/private/SkTHash.h"
#include "src/sksl/SkSLErrorReporter.h"
#include "src/sksl/ir/SkSLSymbol.h"
#include <memory>
#include <vector>
namespace SkSL {
class FunctionDeclaration;
@ -58,25 +59,45 @@ public:
return ptr;
}
// Call fn for every symbol in the table. You may not mutate anything.
template <typename Fn>
void foreach(Fn&& fn) const {
fSymbols.foreach(
[&fn](const SymbolKey& key, const Symbol* symbol) { fn(key.fName, symbol); });
}
size_t count() {
return fSymbols.count();
}
const String* takeOwnershipOfString(std::unique_ptr<String> n);
std::unordered_map<StringFragment, Symbol*>::iterator begin();
std::unordered_map<StringFragment, Symbol*>::iterator end();
std::shared_ptr<SymbolTable> fParent;
std::vector<std::unique_ptr<const Symbol>> fOwnedSymbols;
private:
struct SymbolKey {
StringFragment fName;
uint32_t fHash;
bool operator==(const SymbolKey& that) const { return fName == that.fName; }
bool operator!=(const SymbolKey& that) const { return fName != that.fName; }
struct Hash {
uint32_t operator()(const SymbolKey& key) const { return key.fHash; }
};
};
static SymbolKey MakeSymbolKey(StringFragment name) {
return SymbolKey{name, SkOpts::hash_fn(name.data(), name.size(), 0)};
}
Symbol* lookup(const SymbolKey& key);
static std::vector<const FunctionDeclaration*> GetFunctions(const Symbol& s);
std::vector<std::unique_ptr<IRNode>> fOwnedNodes;
std::vector<std::unique_ptr<String>> fOwnedStrings;
std::unordered_map<StringFragment, Symbol*> fSymbols;
SkTHashMap<SymbolKey, Symbol*, SymbolKey::Hash> fSymbols;
ErrorReporter& fErrorReporter;
friend class Dehydrator;