1) With giving to RegExpCache::create an extra iterator parameter we can simplify some hash lookups. 2) We can collapse the two version of RegExp::create into one, in which we check whether flags are given or not.
Created attachment 59496 [details] Simplify hash lookups in RegExp caching
Comment on attachment 59496 [details] Simplify hash lookups in RegExp caching > +PassRefPtr<RegExp> RegExpCache::create(const UString& patternString, const UString& flags, HashMap<RegExpKey, RefPtr<RegExp> >::iterator iterator) The RegExpCacheMap::iterator will look better. > -PassRefPtr<RegExp> RegExp::create(JSGlobalData* globalData, const UString& pattern) > -{ > - return adoptRef(new RegExp(globalData, pattern)); > -} > - > PassRefPtr<RegExp> RegExp::create(JSGlobalData* globalData, const UString& pattern, const UString& flags) > { > + if (flags.isNull()) > + return adoptRef(new RegExp(globalData, pattern)); > return adoptRef(new RegExp(globalData, pattern, flags)); > } If you propose this merge, what is the reason not to merge the RegExp constructors and create functions?
> The RegExpCacheMap::iterator will look better. You're right. This way is really better. > > -PassRefPtr<RegExp> RegExp::create(JSGlobalData* globalData, const UString& pattern) > > -{ > > - return adoptRef(new RegExp(globalData, pattern)); > > -} > > - > > PassRefPtr<RegExp> RegExp::create(JSGlobalData* globalData, const UString& pattern, const UString& flags) > > { > > + if (flags.isNull()) > > + return adoptRef(new RegExp(globalData, pattern)); > > return adoptRef(new RegExp(globalData, pattern, flags)); > > } > > If you propose this merge, what is the reason not to merge the RegExp constructors and create functions? I agree that we should merge RegExp constructors. But I think it's more prefered to call the public RegExp constructor from the public create function. I have seen examples for this, but maybe I'm wrong. Any comments?
Created attachment 59657 [details] Simplify hash lookups in RegExp caching
> ... to call the public RegExp constructor from the public create function. ... to call the *private* RegExp constructor from the public create function.
Comment on attachment 59657 [details] Simplify hash lookups in RegExp caching r=me Nice cleanup.
http://trac.webkit.org/changeset/61833 might have broken GTK Linux 64-bit Debug
Reopen because it was rolled out by http://trac.webkit.org/changeset/61845
Comment on attachment 59657 [details] Simplify hash lookups in RegExp caching Clearing flags on attachment: 59657 Committed r61924: <http://trac.webkit.org/changeset/61924>
All reviewed patches have been landed. Closing bug.
Rolled out and reopened again. Sorry, I missed to remove cq+ after the previous roll-out. :( Zoltan, if you would like to land cq+ -ed patch manually, next time please set cq- before it, and add comment to the bug with the revision number.
Or you can use "webkit-patch land" which will take care of all that for you.
The reason this patch was crashing: In the case where the cache is full, it reuses the insertion iterator after evicting an item. You can't modify the table while maintaining an iterator into the table.
Created attachment 59920 [details] Patch
Created attachment 59921 [details] Patch
Comment on attachment 59921 [details] Patch r=me
Committed revision 62405.