Bug 194438 - [Cocoa] Optimize ResourceResponse::platformLazyInit()
Summary: [Cocoa] Optimize ResourceResponse::platformLazyInit()
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Chris Dumez
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-02-08 09:16 PST by Chris Dumez
Modified: 2019-02-08 16:27 PST (History)
6 users (show)

See Also:


Attachments
Patch (5.60 KB, patch)
2019-02-08 09:19 PST, Chris Dumez
no flags Details | Formatted Diff | Diff

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