[Webkit-unassigned] [Bug 38142] RegExp caching

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=38142


Eric Seidel <eric at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #54334|review?                     |review-
               Flag|                            |




--- Comment #6 from Eric Seidel <eric at webkit.org>  2010-05-02 18:57:35 PST ---
(From update of attachment 54334)
I would have called "get" "lookupOrCreate" to match the Create pattern of the
rest of the code.

I assume there is no gotchas to making JSGlobalData larger?

+    RefPtr<RegExp> cacheRegExp;
should be cachedRegExp, not "cache"

I think it would be cleaner to move all the creation logic (under the if
(!cacheRegExp) block) into its own function.  Then the function reads:

if (!cachedRegExp)
     return createAndCache(flags, patternString);
return cacheRegExp.release()

Should be m_
+    , ptr(-1) 
+    , isFull(false) 

This code fails for regexps over maxCacheablePatternLength in length.  It
should early return creating a new one each time.

I would have made a countFlags(flags) method for:
+    if (flags.find('g') != UString::NotFound)
+        flagsCnt += 4;
+    if (flags.find('i') != UString::NotFound)
+        flagsCnt += 2;
+    if (flags.find('m') != UString::NotFound)
+        flagsCnt += 1;

Also, countFlags probably goes on RegExpKeyType.

Why RegExpKeyType instead of just RegExpKey?

I *really* like this idea.  I just think the code needs some refinement.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.