Changeset 241220 in webkit

Timestamp:

Feb 8, 2019 4:26:40 PM (5 years ago)

Author:

Chris Dumez

Message:

[Cocoa] Optimize ResourceResponse::platformLazyInit()
​https://bugs.webkit.org/show_bug.cgi?id=194438

Reviewed by Alex Christensen.

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):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/network/cocoa/ResourceResponseCocoa.mm (modified) (3 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-02-08  Chris Dumez  <cdumez@apple.com>
+
+        [Cocoa] Optimize ResourceResponse::platformLazyInit()
+        https://bugs.webkit.org/show_bug.cgi?id=194438
+
+        Reviewed by Alex Christensen.
+
+        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-08  Justin Fan  <justin_fan@apple.com>
 

trunk/Source/WebCore/platform/network/cocoa/ResourceResponseCocoa.mm

@@ -138 +138 @@
 }
 
-enum class OnlyCommonHeaders { No, Yes };
-static inline void initializeHTTPHeaders(OnlyCommonHeaders onlyCommonHeaders, NSHTTPURLResponse *httpResponse, HTTPHeaderMap& headersMap)
-{
-    headersMap.clear();
-    auto messageRef = CFURLResponseGetHTTPResponse([httpResponse _CFURLResponse]);
-
+static inline HTTPHeaderMap initializeHTTPHeaders(CFHTTPMessageRef messageRef)
+{
     // 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;
 }
 
@@ -178 +169 @@
     @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) {
@@ -186 +177 @@
             // 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();
         }
-
     }