Make RegExpCache a weak map
Created attachment 95027 [details]
Comment on attachment 95027 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=95027&action=review
It's good to reuse an expression as long as it's alive. But in generational GC, this implementation will cause regexps to live super long, since we'll stop asking "isReachable" after a while. So, number of GC's seen isn't a very good measure of time.
I'd suggest this two-pronged solution:
(1) Once a fixed limit of expressions is reached, the cache should stop keeping anything artificially alive, and return false from "isReachable." This means that expressions will naturally exit the cache as they become unreachable, but will continue to be shared if they are reachable.
(2) When the system comes under executable memory pressure, iterate the cache and throw all regexp code away. The nice thing about this is that executable memory footprint will never get out of hand. (This can be a separate patch.)
> + RegExp* regexp = new (m_globalData) RegExp(m_globalData, patternString, flags);
Could use a comment here explaining that GC might modify m_cacheMap.
> + if (regExp->hasCode() && regExp->gcShouldInvalidateCode())
> + regExp->invalidateCode();
This kind of operation belongs in finalize, not isReachable.
Created attachment 95051 [details]
Created attachment 95055 [details]
Comment on attachment 95055 [details]
View in context: https://bugs.webkit.org/attachment.cgi?id=95055&action=review
> RegExpCacheMap m_cacheMap;
> + int m_nextEntryInStrongCache;
> + WTF::FixedArray<Strong<RegExp>, maxStrongCacheableEntries> m_strongCache;
Since you've called the new cache the strong cache, I'd call the other cache m_weakCache, and a comment for each:
m_strongCache; // Holds a select few regular expressions that have compiled and executed
m_weakCache; // Holds all regular expressions currently live.
Committed r87445: <http://trac.webkit.org/changeset/87445>