Bug 140048 - Clean up OwnPtr in WebCore/loader
Summary: Clean up OwnPtr in WebCore/loader
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Gyuyoung Kim
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-01-02 20:21 PST by Gyuyoung Kim
Modified: 2015-01-06 09:39 PST (History)
4 users (show)

See Also:

Attachments
WIP (5.50 KB, patch)
2015-01-02 20:23 PST, Gyuyoung Kim 		no flags Details | Formatted Diff | Diff
Patch (7.10 KB, patch)
2015-01-04 00:24 PST, Gyuyoung Kim 		no flags Details | Formatted Diff | Diff
Patch (7.70 KB, patch)
2015-01-04 04:25 PST, Gyuyoung Kim 		no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews104 for mac-mountainlion-wk2 (916.99 KB, application/zip)
2015-01-04 05:37 PST, Build Bot 		no flags Details
Patch (9.13 KB, patch)
2015-01-05 19:53 PST, Gyuyoung Kim 		no flags Details | Formatted Diff | Diff
Patch (9.19 KB, patch)
2015-01-06 08:27 PST, Gyuyoung Kim 		buildbot: commit-queue-
Details | Formatted Diff | Diff
Archive of layout-test-results from ews101 for mac-mountainlion (897.91 KB, application/zip)
2015-01-06 09:33 PST, Build Bot 		no flags Details
Archive of layout-test-results from ews104 for mac-mountainlion-wk2 (910.25 KB, application/zip)
2015-01-06 09:39 PST, Build Bot 		no flags Details
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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