Compare cache patterns #287
Description
Per #284 (comment)
This line runs the parser every time set cssText() is called, so caching it here should result in a slight performance improvement.
However, this contradicts what was previously stated in #271 (comment). What do you think?
First, I think doing any extra caching should be done in a separate PR from this one.
Anyway, my position is similar to that previous comment. We should try to balance the number of caches vs. the performance improvement.
I think the best way to balance this would be to benchmark 3 different setups:
- 3 caches: the current set of caches (isValidPropertyValue, parsePropertyValue, and resolveCalc)
- 4 caches: the current set of caches plus a parseCSS cache
- 2 caches: parseCSS and resolveCalc caches only. (isValidPropertyValue and parsePropertyValue rely on the parseCSS cache, but add a bit of their own work which is not cached.)
When we have those numbers we can make a judgement call. Like, if (3) is 90% as good as (1) or (2), then probably (3) is best. But if (2) gives a 100% performance improvement, then we should just use four caches.
Our benchmarks won't be perfect; in particular, as mentioned in #271 (comment), real-world apps might have worse cache access patterns. But we can react to any reports that people send and add them to the benchmarks and reconsider at that time. Until we get such reports, we can treat the benchmarks as our guide, with a small fudge factor like: 10% worse performance is acceptable if it means fewer caches.
What do you think of this approach?
Originally posted by @domenic in #284 (comment)