Merge pull request #2314 from mgreter/perf/memoize-extend

Speed up repeated extends by memoizing calculations

Author: mgreter

src/ast.cpp

@@ -275,7 +275,7 @@ namespace Sass {
       else if ((!l_h && !r_h) ||
                (!l_h && r_h->empty()) ||
                (!r_h && l_h->empty()) ||
-               (*l_h == *r_h))
+               (l_h && r_h && *l_h == *r_h))
       {
         // check combinator after heads
         if (l->combinator() != r->combinator())
@@ -314,7 +314,6 @@ namespace Sass {
     if (const Complex_Selector* cs = Cast<Complex_Selector>(&rhs)) return *this == *cs;
     if (const Compound_Selector* ch = Cast<Compound_Selector>(&rhs)) return *this == *ch;
     throw std::runtime_error("invalid selector base classes to compare");
-    return false;
   }
 
 
@@ -334,7 +333,6 @@ namespace Sass {
     if (const Complex_Selector* cs = Cast<Complex_Selector>(&rhs)) return *this == *cs;
     if (const Compound_Selector* ch = Cast<Compound_Selector>(&rhs)) return *this == *ch;
     throw std::runtime_error("invalid selector base classes to compare");
-    return false;
   }
 
   bool Compound_Selector::operator< (const Selector& rhs) const
@@ -344,7 +342,6 @@ namespace Sass {
     if (const Complex_Selector* cs = Cast<Complex_Selector>(&rhs)) return *this < *cs;
     if (const Compound_Selector* ch = Cast<Compound_Selector>(&rhs)) return *this < *ch;
     throw std::runtime_error("invalid selector base classes to compare");
-    return false;
   }
 
   bool Selector_Schema::operator== (const Selector& rhs) const
@@ -354,7 +351,6 @@ namespace Sass {
     if (const Complex_Selector* cs = Cast<Complex_Selector>(&rhs)) return *this == *cs;
     if (const Compound_Selector* ch = Cast<Compound_Selector>(&rhs)) return *this == *ch;
     throw std::runtime_error("invalid selector base classes to compare");
-    return false;
   }
 
   bool Selector_Schema::operator< (const Selector& rhs) const

src/extend.cpp

@@ -1511,6 +1511,16 @@ namespace Sass {
   };
   Node Extend::extendCompoundSelector(Compound_Selector_Ptr pSelector, CompoundSelectorSet& seen, bool isReplace) {
 
+    /* this turned out to be too much overhead
+       probably due to holding a "Node" object
+    // check if we already extended this selector
+    // we can do this since subset_map is "static"
+    auto memoized = memoizeCompound.find(pSelector);
+    if (memoized != memoizeCompound.end()) {
+      return memoized->second.klone();
+    }
+    */
+
     DEBUG_EXEC(EXTEND_COMPOUND, printCompoundSelector(pSelector, "EXTEND COMPOUND: "))
     // TODO: Ruby has another loop here to skip certain members?
 
@@ -1658,6 +1668,10 @@ namespace Sass {
 
     DEBUG_EXEC(EXTEND_COMPOUND, printCompoundSelector(pSelector, "EXTEND COMPOUND END: "))
 
+    // this turned out to be too much overhead
+    // memory results in a map table - since extending is very expensive
+    // memoizeCompound.insert(std::pair<Compound_Selector_Obj, Node>(pSelector, results));
+
     return results;
   }
 
@@ -1721,6 +1735,13 @@ namespace Sass {
    */
   Node Extend::extendComplexSelector(Complex_Selector_Ptr selector, CompoundSelectorSet& seen, bool isReplace, bool isOriginal) {
 
+    // check if we already extended this selector
+    // we can do this since subset_map is "static"
+    auto memoized = memoizeComplex.find(selector);
+    if (memoized != memoizeComplex.end()) {
+      return memoized->second;
+    }
+
     // convert the input selector to extend node format
     Node complexSelector = complexSelectorToNode(selector);
     DEBUG_PRINTLN(EXTEND_COMPLEX, "EXTEND COMPLEX: " << complexSelector)
@@ -1825,6 +1846,9 @@ namespace Sass {
     DEBUG_PRINTLN(EXTEND_COMPLEX, ">>>>> EXTENDED: " << extendedSelectors)
     DEBUG_PRINTLN(EXTEND_COMPLEX, "EXTEND COMPLEX END: " << complexSelector)
 
+    // memory results in a map table - since extending is very expensive
+    memoizeComplex.insert(std::pair<Complex_Selector_Obj, Node>(selector, flattened));
+
     // return trim(WEAVES)
     return flattened;
   }
@@ -1840,6 +1864,14 @@ namespace Sass {
 
     Selector_List_Obj pNewSelectors = SASS_MEMORY_NEW(Selector_List, pSelectorList->pstate(), pSelectorList->length());
 
+    // check if we already extended this selector
+    // we can do this since subset_map is "static"
+    auto memoized = memoizeList.find(pSelectorList);
+    if (memoized != memoizeList.end()) {
+      extendedSomething = true;
+      return memoized->second;
+    }
+
     extendedSomething = false;
     // process each comlplex selector in the selector list.
     // Find the ones that can be extended by given subset_map.
@@ -1949,6 +1981,10 @@ namespace Sass {
         cur = cur->tail();
       }
     }
+
+    // memory results in a map table - since extending is very expensive
+    memoizeList.insert(std::pair<Selector_List_Obj, Selector_List_Obj>(pSelectorList, pNewSelectors));
+
     return pNewSelectors.detach();
 
   }

src/extend.hpp

@@ -22,6 +22,30 @@ namespace Sass {
 
   private:
 
+    std::unordered_map<
+      Selector_List_Obj, // key
+      Selector_List_Obj, // value
+      HashNodes, // hasher
+      CompareNodes // compare
+    > memoizeList;
+
+    std::unordered_map<
+      Complex_Selector_Obj, // key
+      Node, // value
+      HashNodes, // hasher
+      CompareNodes // compare
+    > memoizeComplex;
+
+    /* this turned out to be too much overhead
+       re-evaluate once we store an ast selector
+    std::unordered_map<
+      Compound_Selector_Obj, // key
+      Node, // value
+      HashNodes, // hasher
+      CompareNodes // compare
+    > memoizeCompound;
+    */
+
     void extendObjectWithSelectorAndBlock(Ruleset_Ptr pObject);
     Node extendComplexSelector(Complex_Selector_Ptr sel, CompoundSelectorSet& seen, bool isReplace, bool isOriginal);
     Node extendCompoundSelector(Compound_Selector_Ptr sel, CompoundSelectorSet& seen, bool isReplace);