SkSL performance improvements

src/sksl/ir/SkSLSymbolTable.cpp

Index: src/sksl/ir/SkSLSymbolTable.cpp
diff --git a/src/sksl/ir/SkSLSymbolTable.cpp b/src/sksl/ir/SkSLSymbolTable.cpp
index af83f7a456241abe4615a92e702d7084da5dc497..359138f5a59bf797126bb7b80501ee1f554c2dcb 100644
--- a/src/sksl/ir/SkSLSymbolTable.cpp
+++ b/src/sksl/ir/SkSLSymbolTable.cpp
@@ -9,19 +9,24 @@
 
 namespace SkSL {
 
-std::vector<std::shared_ptr<FunctionDeclaration>> SymbolTable::GetFunctions(
-                                                                 const std::shared_ptr<Symbol>& s) {
-    switch (s->fKind) {
+SymbolTable::~SymbolTable() {
+    for (Symbol* s : fOwnedPointers) {
+        delete s;
+    }
+}
+
+std::vector<const FunctionDeclaration*> SymbolTable::GetFunctions(const Symbol& s) {
+    switch (s.fKind) {
         case Symbol::kFunctionDeclaration_Kind:
-            return { std::static_pointer_cast<FunctionDeclaration>(s) };
+            return { &((FunctionDeclaration&) s) };
         case Symbol::kUnresolvedFunction_Kind:
-            return ((UnresolvedFunction&) *s).fFunctions;
+            return ((UnresolvedFunction&) s).fFunctions;
         default:
             return { };
     }
 }
 
-std::shared_ptr<Symbol> SymbolTable::operator[](const std::string& name) {
+const Symbol* SymbolTable::operator[](const std::string& name) {
     const auto& entry = fSymbols.find(name);
     if (entry == fSymbols.end()) {
         if (fParent) {
@@ -30,15 +35,15 @@ std::shared_ptr<Symbol> SymbolTable::operator[](const std::string& name) {
         return nullptr;
     }
     if (fParent) {
-        auto functions = GetFunctions(entry->second);
+        auto functions = GetFunctions(*entry->second);
         if (functions.size() > 0) {
             bool modified = false;
-            std::shared_ptr<Symbol> previous = (*fParent)[name];
+            const Symbol* previous = (*fParent)[name];
             if (previous) {
-                auto previousFunctions = GetFunctions(previous);
-                for (const std::shared_ptr<FunctionDeclaration>& prev : previousFunctions) {
+                auto previousFunctions = GetFunctions(*previous);
+                for (const FunctionDeclaration* prev : previousFunctions) {
                     bool found = false;
-                    for (const std::shared_ptr<FunctionDeclaration>& current : functions) {
+                    for (const FunctionDeclaration* current : functions) {
                         if (current->matches(*prev)) {
                             found = true;
                             break;
@@ -51,7 +56,9 @@ std::shared_ptr<Symbol> SymbolTable::operator[](const std::string& name) {
                 }
                 if (modified) {
                     ASSERT(functions.size() > 1);
-                    return std::shared_ptr<Symbol>(new UnresolvedFunction(functions));
+                    UnresolvedFunction* result = new UnresolvedFunction(functions);
+                    this->takeOwnership(result);

[dogben, 2016/07/08 19:56:40]

nit: Maybe takeOwnership could return the raw pointer so this could be "return
this->takeOwnership(result);"

+                    return result;
                 }
             }
         }
@@ -59,27 +66,46 @@ std::shared_ptr<Symbol> SymbolTable::operator[](const std::string& name) {
     return entry->second;
 }
 
-void SymbolTable::add(const std::string& name, std::shared_ptr<Symbol> symbol) {
-        const auto& existing = fSymbols.find(name);
-        if (existing == fSymbols.end()) {
-            fSymbols[name] = symbol;
-        } else if (symbol->fKind == Symbol::kFunctionDeclaration_Kind) {
-            const std::shared_ptr<Symbol>& oldSymbol = existing->second;
-            if (oldSymbol->fKind == Symbol::kFunctionDeclaration_Kind) {
-                std::vector<std::shared_ptr<FunctionDeclaration>> functions;
-                functions.push_back(std::static_pointer_cast<FunctionDeclaration>(oldSymbol));
-                functions.push_back(std::static_pointer_cast<FunctionDeclaration>(symbol));
-                fSymbols[name].reset(new UnresolvedFunction(std::move(functions)));
-            } else if (oldSymbol->fKind == Symbol::kUnresolvedFunction_Kind) {
-                std::vector<std::shared_ptr<FunctionDeclaration>> functions;
-                for (const auto& f : ((UnresolvedFunction&) *oldSymbol).fFunctions) {
-                    functions.push_back(f);
-                }
-                functions.push_back(std::static_pointer_cast<FunctionDeclaration>(symbol));
-                fSymbols[name].reset(new UnresolvedFunction(std::move(functions)));
+void SymbolTable::takeOwnership(Symbol* s) {
+    if (fOwner) {
+        fOwnedPointers.push_back(s);
+    } else {
+        ASSERT(fParent);
+        fParent->takeOwnership(s);
+    }
+}
+
+void SymbolTable::add(const std::string& name, Symbol* symbol) {
+    this->addWithoutOwnership(name, symbol);
+    this->takeOwnership(symbol);
+}
+
+void SymbolTable::addWithoutOwnership(const std::string& name, const Symbol* symbol) {
+    const auto& existing = fSymbols.find(name);
+    if (existing == fSymbols.end()) {
+        fSymbols[name] = symbol;
+    } else if (symbol->fKind == Symbol::kFunctionDeclaration_Kind) {
+        const Symbol* oldSymbol = existing->second;
+        if (oldSymbol->fKind == Symbol::kFunctionDeclaration_Kind) {
+            std::vector<const FunctionDeclaration*> functions;
+            functions.push_back((const FunctionDeclaration*) oldSymbol);
+            functions.push_back((const FunctionDeclaration*) symbol);
+            UnresolvedFunction* u = new UnresolvedFunction(std::move(functions));
+            fSymbols[name] = u;
+            this->takeOwnership(u);
+        } else if (oldSymbol->fKind == Symbol::kUnresolvedFunction_Kind) {
+            std::vector<const FunctionDeclaration*> functions;
+            for (const auto& f : ((UnresolvedFunction&) *oldSymbol).fFunctions) {

[dogben, 2016/07/08 19:56:40]

nit: s/const auto&/const auto*/

+                functions.push_back(f);
             }
-        } else {
-            fErrorReporter.error(symbol->fPosition, "symbol '" + name + "' was already defined");
+            functions.push_back((const FunctionDeclaration*) symbol);
+            UnresolvedFunction* u = new UnresolvedFunction(std::move(functions));
+            fSymbols[name] = u;
+            this->takeOwnership(u);
         }
+    } else {
+        fErrorReporter.error(symbol->fPosition, "symbol '" + name + "' was already defined");
     }
+}
+
 } // namespace