Attachment 361509 Details for Bug 194438 – Patch

[patch] Patch

bug-194438-20190208091941.patch (text/plain), 5.60 KB, created by Chris Dumez on 2019-02-08 09:19:42 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Chris Dumez

Created: 2019-02-08 09:19:42 PST

Size: 5.60 KB

patch

obsolete

>Subversion Revision: 241026
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 33d6584bfde7103e976f58b1fc02237b35e53aa0..9e5408fcd5095e0cfad2b8de38d11ff78fcfe68d 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,31 @@
>+2019-02-08  Chris Dumez  <cdumez@apple.com>
>+
>+        [Cocoa] Optimize ResourceResponse::platformLazyInit()
>+        https://bugs.webkit.org/show_bug.cgi?id=194438
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Optimize ResourceResponse::platformLazyInit(). Most of the CPU time currently goes into getting the
>+        HTTP headers from CFNetwork:
>+        """
>+        Sample Count, Samples %, CPU %, Symbol
>+        46, 0.0%, 0.0%, WebCore::initializeHTTPHeaders(WebCore::OnlyCommonHeaders, NSHTTPURLResponse*, WebCore::HTTPHeaderMap&) (in WebCore)
>+        34, 0.0%, 0.0%,     HTTPHeaderDict::copyAsOrdinaryDict(__CFAllocator const*) const (in CFNetwork)
>+        11, 0.0%, 0.0%,     CFDictionaryApplyFunction (in CoreFoundation)
>+        """
>+
>+        We currently have 2 levels of initialization: CommonFieldsOnly & AllFields. With WebKit2, most ResourceResponses get sent over IPC
>+        and thus end up getting initialized twice, once with CommonFieldsOnly and then with AllFields.
>+        This would cause us to call the expensive HTTPHeaderDict::copyAsOrdinaryDict() twice instead of once, simply to initialize the common
>+        HTTP headers first and then the uncommon ones later.
>+
>+        This patch updates ResourceResponse::platformLazyInit() to initialize all HTTP headers at once, as soon as CommonFieldsOnly
>+        initialization is requested, so that we no longer copy all HTTP headers twice.
>+
>+        * platform/network/cocoa/ResourceResponseCocoa.mm:
>+        (WebCore::initializeHTTPHeaders):
>+        (WebCore::ResourceResponse::platformLazyInit):
>+
> 2019-02-06  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][IFC] Move line layout code to a dedicated file
>diff --git a/Source/WebCore/platform/network/cocoa/ResourceResponseCocoa.mm b/Source/WebCore/platform/network/cocoa/ResourceResponseCocoa.mm
>index f6e242bd1d451b26f73d1ef77d492a65c9617241..16f11d11763514e711ebbac9d7b896a616b33f0c 100644
>--- a/Source/WebCore/platform/network/cocoa/ResourceResponseCocoa.mm
>+++ b/Source/WebCore/platform/network/cocoa/ResourceResponseCocoa.mm
>@@ -137,23 +137,14 @@ static inline AtomicString stripLeadingAndTrailingDoubleQuote(const String& valu
>     return StringView(value).substring(1, length - 2).toAtomicString();
> }
> 
>-enum class OnlyCommonHeaders { No, Yes };
>-static inline void initializeHTTPHeaders(OnlyCommonHeaders onlyCommonHeaders, NSHTTPURLResponse *httpResponse, HTTPHeaderMap& headersMap)
>+static inline HTTPHeaderMap initializeHTTPHeaders(CFHTTPMessageRef messageRef)
> {
>-    headersMap.clear();
>-    auto messageRef = CFURLResponseGetHTTPResponse([httpResponse _CFURLResponse]);
>-
>     // Avoid calling [NSURLResponse allHeaderFields] to minimize copying (<rdar://problem/26778863>).
>     auto headers = adoptCF(CFHTTPMessageCopyAllHeaderFields(messageRef));
>-    if (onlyCommonHeaders == OnlyCommonHeaders::Yes) {
>-        for (auto& commonHeader : commonHeaderFields) {
>-            const void* value;
>-            if (CFDictionaryGetValueIfPresent(headers.get(), commonHeader, &value))
>-                headersMap.set(commonHeader, (CFStringRef) value);
>-        }
>-        return;
>-    }
>+
>+    HTTPHeaderMap headersMap;
>     CFDictionaryApplyFunction(headers.get(), addToHTTPHeaderMap, &headersMap);
>+    return headersMap;
> }
> 
> static inline AtomicString extractHTTPStatusText(CFHTTPMessageRef messageRef)
>@@ -177,7 +168,7 @@ void ResourceResponse::platformLazyInit(InitLevel initLevel)
>     
>     @autoreleasepool {
> 
>-        NSHTTPURLResponse *httpResponse = [m_nsResponse.get() isKindOfClass:[NSHTTPURLResponse class]] ? (NSHTTPURLResponse *)m_nsResponse.get() : nullptr;
>+        auto messageRef = [m_nsResponse.get() isKindOfClass:[NSHTTPURLResponse class]] ? CFURLResponseGetHTTPResponse([ (NSHTTPURLResponse *)m_nsResponse.get() _CFURLResponse]) : nullptr;
> 
>         if (m_initLevel < CommonFieldsOnly) {
>             m_url = [m_nsResponse.get() URL];
>@@ -185,18 +176,14 @@ void ResourceResponse::platformLazyInit(InitLevel initLevel)
>             m_expectedContentLength = [m_nsResponse.get() expectedContentLength];
>             // Stripping double quotes as a workaround for <rdar://problem/8757088>, can be removed once that is fixed.
>             m_textEncodingName = stripLeadingAndTrailingDoubleQuote([m_nsResponse.get() textEncodingName]);
>-            m_httpStatusCode = httpResponse ? [httpResponse statusCode] : 0;
>+            m_httpStatusCode = messageRef ? CFHTTPMessageGetResponseStatusCode(messageRef) : 0;
>+            if (messageRef)
>+                m_httpHeaderFields = initializeHTTPHeaders(messageRef);
>         }
>-        if (httpResponse) {
>-            if (initLevel == AllFields) {
>-                auto messageRef = CFURLResponseGetHTTPResponse([httpResponse _CFURLResponse]);
>-                m_httpStatusText = extractHTTPStatusText(messageRef);
>-                m_httpVersion = String(adoptCF(CFHTTPMessageCopyVersion(messageRef)).get()).convertToASCIIUppercase();
>-                initializeHTTPHeaders(OnlyCommonHeaders::No, httpResponse, m_httpHeaderFields);
>-            } else
>-                initializeHTTPHeaders(OnlyCommonHeaders::Yes, httpResponse, m_httpHeaderFields);
>+        if (messageRef && initLevel == AllFields) {
>+            m_httpStatusText = extractHTTPStatusText(messageRef);
>+            m_httpVersion = String(adoptCF(CFHTTPMessageCopyVersion(messageRef)).get()).convertToASCIIUppercase();
>         }
>-
>     }
> 
>     m_initLevel = initLevel;

Actions: View | Formatted Diff | Diff

Attachments on bug 194438: 361509