Skip to content

Commit ec98942

Browse files
committed
Speed up repeated extends by memoizing calculations
The extend code path is probably the most expensive to go down in LibSass. It involves multiple conversions between the regular AST selector node to the internal used format. Storing results from previous runs is rather cheap, due to the new shared object memory storage and since we already have everything to use selectors efficiently as map keys. It turned out that memoizing Compound-Selectors was too much overhead. This is probably due to having a `Node` object a map value (IMO this is a copy assignment). I commented it out for now with some info for later re- evaluation when we can store a more efficient value!
1 parent e4e65b4 commit ec98942

File tree

3 files changed

+61
-5
lines changed

3 files changed

+61
-5
lines changed

src/ast.cpp

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -275,7 +275,7 @@ namespace Sass {
275275
else if ((!l_h && !r_h) ||
276276
(!l_h && r_h->empty()) ||
277277
(!r_h && l_h->empty()) ||
278-
(*l_h == *r_h))
278+
(l_h && r_h && *l_h == *r_h))
279279
{
280280
// check combinator after heads
281281
if (l->combinator() != r->combinator())
@@ -314,7 +314,6 @@ namespace Sass {
314314
if (const Complex_Selector* cs = Cast<Complex_Selector>(&rhs)) return *this == *cs;
315315
if (const Compound_Selector* ch = Cast<Compound_Selector>(&rhs)) return *this == *ch;
316316
throw std::runtime_error("invalid selector base classes to compare");
317-
return false;
318317
}
319318

320319

@@ -334,7 +333,6 @@ namespace Sass {
334333
if (const Complex_Selector* cs = Cast<Complex_Selector>(&rhs)) return *this == *cs;
335334
if (const Compound_Selector* ch = Cast<Compound_Selector>(&rhs)) return *this == *ch;
336335
throw std::runtime_error("invalid selector base classes to compare");
337-
return false;
338336
}
339337

340338
bool Compound_Selector::operator< (const Selector& rhs) const
@@ -344,7 +342,6 @@ namespace Sass {
344342
if (const Complex_Selector* cs = Cast<Complex_Selector>(&rhs)) return *this < *cs;
345343
if (const Compound_Selector* ch = Cast<Compound_Selector>(&rhs)) return *this < *ch;
346344
throw std::runtime_error("invalid selector base classes to compare");
347-
return false;
348345
}
349346

350347
bool Selector_Schema::operator== (const Selector& rhs) const
@@ -354,7 +351,6 @@ namespace Sass {
354351
if (const Complex_Selector* cs = Cast<Complex_Selector>(&rhs)) return *this == *cs;
355352
if (const Compound_Selector* ch = Cast<Compound_Selector>(&rhs)) return *this == *ch;
356353
throw std::runtime_error("invalid selector base classes to compare");
357-
return false;
358354
}
359355

360356
bool Selector_Schema::operator< (const Selector& rhs) const

src/extend.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1511,6 +1511,16 @@ namespace Sass {
15111511
};
15121512
Node Extend::extendCompoundSelector(Compound_Selector_Ptr pSelector, CompoundSelectorSet& seen, bool isReplace) {
15131513

1514+
/* this turned out to be too much overhead
1515+
probably due to holding a "Node" object
1516+
// check if we already extended this selector
1517+
// we can do this since subset_map is "static"
1518+
auto memoized = memoizeCompound.find(pSelector);
1519+
if (memoized != memoizeCompound.end()) {
1520+
return memoized->second.klone();
1521+
}
1522+
*/
1523+
15141524
DEBUG_EXEC(EXTEND_COMPOUND, printCompoundSelector(pSelector, "EXTEND COMPOUND: "))
15151525
// TODO: Ruby has another loop here to skip certain members?
15161526

@@ -1658,6 +1668,10 @@ namespace Sass {
16581668

16591669
DEBUG_EXEC(EXTEND_COMPOUND, printCompoundSelector(pSelector, "EXTEND COMPOUND END: "))
16601670

1671+
// this turned out to be too much overhead
1672+
// memory results in a map table - since extending is very expensive
1673+
// memoizeCompound.insert(std::pair<Compound_Selector_Obj, Node>(pSelector, results));
1674+
16611675
return results;
16621676
}
16631677

@@ -1721,6 +1735,13 @@ namespace Sass {
17211735
*/
17221736
Node Extend::extendComplexSelector(Complex_Selector_Ptr selector, CompoundSelectorSet& seen, bool isReplace, bool isOriginal) {
17231737

1738+
// check if we already extended this selector
1739+
// we can do this since subset_map is "static"
1740+
auto memoized = memoizeComplex.find(selector);
1741+
if (memoized != memoizeComplex.end()) {
1742+
return memoized->second;
1743+
}
1744+
17241745
// convert the input selector to extend node format
17251746
Node complexSelector = complexSelectorToNode(selector);
17261747
DEBUG_PRINTLN(EXTEND_COMPLEX, "EXTEND COMPLEX: " << complexSelector)
@@ -1825,6 +1846,9 @@ namespace Sass {
18251846
DEBUG_PRINTLN(EXTEND_COMPLEX, ">>>>> EXTENDED: " << extendedSelectors)
18261847
DEBUG_PRINTLN(EXTEND_COMPLEX, "EXTEND COMPLEX END: " << complexSelector)
18271848

1849+
// memory results in a map table - since extending is very expensive
1850+
memoizeComplex.insert(std::pair<Complex_Selector_Obj, Node>(selector, flattened));
1851+
18281852
// return trim(WEAVES)
18291853
return flattened;
18301854
}
@@ -1840,6 +1864,14 @@ namespace Sass {
18401864

18411865
Selector_List_Obj pNewSelectors = SASS_MEMORY_NEW(Selector_List, pSelectorList->pstate(), pSelectorList->length());
18421866

1867+
// check if we already extended this selector
1868+
// we can do this since subset_map is "static"
1869+
auto memoized = memoizeList.find(pSelectorList);
1870+
if (memoized != memoizeList.end()) {
1871+
extendedSomething = true;
1872+
return memoized->second;
1873+
}
1874+
18431875
extendedSomething = false;
18441876
// process each comlplex selector in the selector list.
18451877
// Find the ones that can be extended by given subset_map.
@@ -1949,6 +1981,10 @@ namespace Sass {
19491981
cur = cur->tail();
19501982
}
19511983
}
1984+
1985+
// memory results in a map table - since extending is very expensive
1986+
memoizeList.insert(std::pair<Selector_List_Obj, Selector_List_Obj>(pSelectorList, pNewSelectors));
1987+
19521988
return pNewSelectors.detach();
19531989

19541990
}

src/extend.hpp

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,30 @@ namespace Sass {
2222

2323
private:
2424

25+
std::unordered_map<
26+
Selector_List_Obj, // key
27+
Selector_List_Obj, // value
28+
HashNodes, // hasher
29+
CompareNodes // compare
30+
> memoizeList;
31+
32+
std::unordered_map<
33+
Complex_Selector_Obj, // key
34+
Node, // value
35+
HashNodes, // hasher
36+
CompareNodes // compare
37+
> memoizeComplex;
38+
39+
/* this turned out to be too much overhead
40+
re-evaluate once we store an ast selector
41+
std::unordered_map<
42+
Compound_Selector_Obj, // key
43+
Node, // value
44+
HashNodes, // hasher
45+
CompareNodes // compare
46+
> memoizeCompound;
47+
*/
48+
2549
void extendObjectWithSelectorAndBlock(Ruleset_Ptr pObject);
2650
Node extendComplexSelector(Complex_Selector_Ptr sel, CompoundSelectorSet& seen, bool isReplace, bool isOriginal);
2751
Node extendCompoundSelector(Compound_Selector_Ptr sel, CompoundSelectorSet& seen, bool isReplace);

0 commit comments

Comments
 (0)