Bug 113469 - Safari hangs during test262 run in CodeCache::pruneSlowCase
Summary: Safari hangs during test262 run in CodeCache::pruneSlowCase
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Mark Hahnenberg
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-03-27 19:59 PDT by Mark Hahnenberg
Modified: 2013-03-28 13:19 PDT (History)
1 user (show)

See Also:


Attachments
Patch (3.62 KB, patch)
2013-03-28 12:11 PDT, Mark Hahnenberg
no flags Details | Formatted Diff | Diff
Patch (3.93 KB, patch)
2013-03-28 12:46 PDT, Mark Hahnenberg
ggaren: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Hahnenberg 2013-03-27 19:59:05 PDT
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.
Comment 1 Mark Hahnenberg 2013-03-28 12:11:04 PDT
Created attachment 195613 [details]
Patch
Comment 2 Mark Hahnenberg 2013-03-28 12:19:01 PDT
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?
Comment 3 Mark Hahnenberg 2013-03-28 12:36:32 PDT
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.
Comment 4 Mark Hahnenberg 2013-03-28 12:46:12 PDT
Created attachment 195622 [details]
Patch
Comment 5 Geoffrey Garen 2013-03-28 13:04:17 PDT
Comment on attachment 195622 [details]
Patch

r=me
Comment 6 Geoffrey Garen 2013-03-28 13:04:58 PDT
> the resulting behavior is still a regression in performance over shipping Safari

Is this still the case in your latest patch? Regression on what?
Comment 7 Mark Hahnenberg 2013-03-28 13:10:31 PDT
(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.
Comment 8 Mark Hahnenberg 2013-03-28 13:13:52 PDT
(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.
Comment 9 Mark Hahnenberg 2013-03-28 13:19:21 PDT
Committed r147150: <http://trac.webkit.org/changeset/147150>