Bug 61554 – Make RegExpCache a weak map

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Oliver Hunt]

Make RegExpCache a weak map

[Oliver Hunt]

Created attachment 95027 [details]
Patch

[Geoffrey Garen]

Comment on attachment 95027 [details]
Patch

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.)

> Source/JavaScriptCore/runtime/RegExpCache.cpp:41
> +    RegExp* regexp = new (m_globalData) RegExp(m_globalData, patternString, flags);

Could use a comment here explaining that GC might modify m_cacheMap.

> Source/JavaScriptCore/runtime/RegExpCache.cpp:58
> +    if (regExp->hasCode() && regExp->gcShouldInvalidateCode())
> +        regExp->invalidateCode();

This kind of operation belongs in finalize, not isReachable.

[Oliver Hunt]

Created attachment 95051 [details]
Patch

[Oliver Hunt]

Created attachment 95055 [details]
Patch

[Geoffrey Garen]

Comment on attachment 95055 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=95055&action=review

r=me

> Source/JavaScriptCore/runtime/RegExpCache.h:60
>      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.

[Oliver Hunt]

Committed r87445: <http://trac.webkit.org/changeset/87445>