Bug 140048 – Clean up OwnPtr in WebCore/loader

• Status: NEW
• Component: WebCore Misc.

[Gyuyoung Kim]

SSIA

[Gyuyoung Kim]

Created attachment 243913 [details]
WIP

[Gyuyoung Kim]

Created attachment 243924 [details]
Patch

[Gyuyoung Kim]

Created attachment 243927 [details]
Patch

[Build Bot]

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.

[Build Bot]

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

[Darin Adler]

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.

[Gyuyoung Kim]

Created attachment 244029 [details]
Patch

[Gyuyoung Kim]

Created attachment 244063 [details]
Patch

[Build Bot]

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.

[Build Bot]

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

[Build Bot]

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.

[Build Bot]

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