Bug 194438

Summary: [Cocoa] Optimize ResourceResponse::platformLazyInit()
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: WebKit2Assignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, beidson, commit-queue, ggaren, rniwa, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch none

Description Chris Dumez 2019-02-08 09:16:38 PST
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.

My proposal is to initialize all HTTP headers once, as soon as CommonFieldsOnly initialized is requested, so that we no longer copy all HTTP headers twice.
Comment 1 Chris Dumez 2019-02-08 09:19:42 PST
Created attachment 361509 [details]
Patch
Comment 2 Alex Christensen 2019-02-08 15:34:45 PST
Comment on attachment 361509 [details]
Patch

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

> Source/WebCore/ChangeLog:22
> +        This patch updates ResourceResponse::platformLazyInit() to initialize all HTTP headers at once, as soon as CommonFieldsOnly

If we're doing this, why not just remove CommonFieldsOnly?
Comment 3 Chris Dumez 2019-02-08 15:48:29 PST
(In reply to Alex Christensen from comment #2)
> Comment on attachment 361509 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=361509&action=review
> 
> > Source/WebCore/ChangeLog:22
> > +        This patch updates ResourceResponse::platformLazyInit() to initialize all HTTP headers at once, as soon as CommonFieldsOnly
> 
> If we're doing this, why not just remove CommonFieldsOnly?

I would need to gather more data before doing such change. Also, this is likely not true for WebKit1 since no IPC.
Comment 4 Chris Dumez 2019-02-08 16:18:40 PST
Comment on attachment 361509 [details]
Patch

In a follow-up, I will look into removing the double stage lazy init altogether.
Comment 5 WebKit Commit Bot 2019-02-08 16:26:46 PST
Comment on attachment 361509 [details]
Patch

Clearing flags on attachment: 361509

Committed r241220: <https://trac.webkit.org/changeset/241220>
Comment 6 WebKit Commit Bot 2019-02-08 16:26:47 PST
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2019-02-08 16:27:28 PST
<rdar://problem/47934216>