Safari hangs during test262 run in CodeCache::pruneSlowCase

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Mark Hahnenberg]

I added some logging which shows that we hang for a really long time in pruneSlowCase removing many very small entries from the map. Most of the short keys appear to be regular expressions with 16-bit characters (e.g. Chinese characters). Maybe having a limit on the number of entries in combination with a limit on the total size of all entries in the map would be a better solution than just size alone to prevent these long hangs churning on pruning many small objects from the cache.

[Mark Hahnenberg]

Created attachment 195613 [details]
Patch

[Mark Hahnenberg]

Comment on attachment 195613 [details]
Patch

Hmm, this patch is an improvement, but the resulting behavior is still a regression in performance over shipping Safari. I think we're hitting the prune case a lot (add 1 element, remove 1 element). Maybe it makes more sense to coalesce a lot of calls to m_map.remove() into a single call to pruneSlowCase?

[Mark Hahnenberg]

Comment on attachment 195613 [details]
Patch

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

> Source/JavaScriptCore/runtime/CodeCache.cpp:50
> +    while (m_size > m_capacity && numberOfEntries() >= workingSetMaxEntries) {

This is wrong.

[Mark Hahnenberg]

Created attachment 195622 [details]
Patch

[Geoffrey Garen]

Comment on attachment 195622 [details]
Patch

r=me

[Geoffrey Garen]

> the resulting behavior is still a regression in performance over shipping Safari

Is this still the case in your latest patch? Regression on what?

[Mark Hahnenberg]

(In reply to comment #6)
> > the resulting behavior is still a regression in performance over shipping Safari
> 
> Is this still the case in your latest patch? Regression on what?

Nope, it behaves the same now.

[Mark Hahnenberg]

(In reply to comment #7)
> (In reply to comment #6)
> > > the resulting behavior is still a regression in performance over shipping Safari
> > 
> > Is this still the case in your latest patch? Regression on what?
> 
> Nope, it behaves the same now.

Sorry, the regression was in performance. The first patch was better in performance than on ToT, but it would still hang a little. The latest patch's performance matches that of shipping Safari.

[Mark Hahnenberg]

Committed r147150: <http://trac.webkit.org/changeset/147150>