Bug 140048

Summary: Clean up OwnPtr in WebCore/loader
Product: WebKit Reporter: Gyuyoung Kim <gyuyoung.kim>
Component: WebCore Misc.Assignee: Gyuyoung Kim <gyuyoung.kim>
Status: NEW ---    
Severity: Normal CC: buildbot, commit-queue, japhet, rniwa
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
WIP
none
Patch
none
Patch
none
Archive of layout-test-results from ews104 for mac-mountainlion-wk2
none
Patch
none
Patch
buildbot: commit-queue-
Archive of layout-test-results from ews101 for mac-mountainlion
none
Archive of layout-test-results from ews104 for mac-mountainlion-wk2 none

Description Gyuyoung Kim 2015-01-02 20:21:47 PST
SSIA
Comment 1 Gyuyoung Kim 2015-01-02 20:23:38 PST
Created attachment 243913 [details]
WIP
Comment 2 Gyuyoung Kim 2015-01-04 00:24:48 PST
Created attachment 243924 [details]
Patch
Comment 3 Gyuyoung Kim 2015-01-04 04:25:46 PST
Created attachment 243927 [details]
Patch
Comment 4 Build Bot 2015-01-04 05:37:31 PST
Comment on attachment 243927 [details]
Patch

Attachment 243927 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6523443113623552

Number of test failures exceeded the failure limit.
Comment 5 Build Bot 2015-01-04 05:37:33 PST
Created attachment 243928 [details]
Archive of layout-test-results from ews104 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5
Comment 6 Darin Adler 2015-01-04 13:57:49 PST
Comment on attachment 243927 [details]
Patch

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

> Source/WebCore/loader/cache/MemoryCache.cpp:119
>      if (!originMap) {
> -        originMap = new CachedResourceItem;
> -        resources.set(resource->url(), adoptPtr(originMap));
> +        resources.set(resource->url(), std::make_unique<CachedResourceItem>());
> +        originMap = resources.get(resource->url());
>      }
>      originMap->set(resource->cachePartition(), resource);

This does two extra hash table lookups the first time it populates the resources map for any given URL, bloating both code size and execution speed. The existing code was already doing one extra hash lookup, but this patch added yet another. The correct way to write this is:

    auto& originMap = resources.add(resource->url(), nullptr)->iterator.value;
    if (!originMap)
        originMap = std::make_unique<CachedResourceItem>();
    originMap->set(resource->cachePartition(), resource);

> Source/WebCore/loader/cache/MemoryCache.cpp:154
>      CachedResourceItem* originMap = resources.get(resource->url());
>      if (!originMap) {
> -        originMap = new CachedResourceItem;
> -        resources.set(resource->url(), adoptPtr(originMap));
> +        resources.set(resource->url(), std::make_unique<CachedResourceItem>());
> +        originMap = resources.get(resource->url());
>      }
>      originMap->set(resource->cachePartition(), resource);

Ditto.
Comment 7 Gyuyoung Kim 2015-01-05 19:53:29 PST
Created attachment 244029 [details]
Patch
Comment 8 Gyuyoung Kim 2015-01-06 08:27:30 PST
Created attachment 244063 [details]
Patch
Comment 9 Build Bot 2015-01-06 09:33:35 PST
Comment on attachment 244063 [details]
Patch

Attachment 244063 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/6334210612658176

Number of test failures exceeded the failure limit.
Comment 10 Build Bot 2015-01-06 09:33:38 PST
Created attachment 244067 [details]
Archive of layout-test-results from ews101 for mac-mountainlion

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews101  Port: mac-mountainlion  Platform: Mac OS X 10.8.5
Comment 11 Build Bot 2015-01-06 09:39:37 PST
Comment on attachment 244063 [details]
Patch

Attachment 244063 [details] did not pass mac-wk2-ews (mac-wk2):
Output: http://webkit-queues.appspot.com/results/6682011561361408

Number of test failures exceeded the failure limit.
Comment 12 Build Bot 2015-01-06 09:39:40 PST
Created attachment 244069 [details]
Archive of layout-test-results from ews104 for mac-mountainlion-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews104  Port: mac-mountainlion-wk2  Platform: Mac OS X 10.8.5