Bug 137367

Summary: [Mac] We are spending too much time serializing ProtectionSpace objects
Product: WebKit Reporter: Chris Dumez <cdumez>
Component: Page LoadingAssignee: Chris Dumez <cdumez>
Status: RESOLVED FIXED    
Severity: Enhancement CC: andersca, ap, barraclough, beidson, commit-queue, mitz
Priority: P2 Keywords: InRadar
Version: 528+ (Nightly build)   
Hardware: Mac   
OS: OS X 10.9   
Attachments:
Description Flags
WIP Patch
none
WIP Patch
none
Patch
none
Patch
none
Patch none

Description Chris Dumez 2014-10-02 16:22:04 PDT
When profiling the load of nytimes.com, I noticed that we were spending a lot of cpu time serializing ProtectionSpace objects (in particular the NSURLProtectionSpace platform data):
- 5.6% of CPU time for Network Process
- 2.5% of CPU time for WebProcess

Serializing an NSURLProtectionSpace seems to be costly due to server trust verification. We do this for every sub-resource load over HTTPS due to the canAuthenticateAgainstProtectionSpace() callback for server trust validation, from the NetworkProcess to the WebProcess and then to the UIProcess:

After discussing with Dan, a possibility for the short-term would be to add a "LetPlatformHandleServerTrustValidation" setting so that we can let CFNetwork handle the server trust validation (this is what ends up happening anyway currently), to avoid doing all the IPC (and thus serialization) unnecessarily for server trust validation, if the client does not handle it.
Comment 1 Chris Dumez 2014-10-02 16:37:36 PDT
I was thinking "setShouldHandleHTTPSServerTrustEvaluationAtNetworkLevel(bool)" for the (private) setting. Any thoughts?
Comment 2 Chris Dumez 2014-10-02 17:06:58 PDT
Created attachment 239158 [details]
WIP Patch
Comment 3 Chris Dumez 2014-10-02 17:09:17 PDT
<rdar://problem/18516890>
Comment 4 Chris Dumez 2014-10-02 17:10:56 PDT
Created attachment 239159 [details]
WIP Patch
Comment 5 Chris Dumez 2014-10-03 10:37:35 PDT
Created attachment 239210 [details]
Patch
Comment 6 mitz 2014-10-03 10:56:36 PDT
Just a couple of comments:
1. If a Networking process crashes and the WebContext launches a new one to replace it, it will lose this state. To fix this, the WebContext needs to keep the flag and include it in the initialization parameters to the Networking process.
2. This should also be exposed as WKProcessPool SPI so that it can be used by clients of that API.
Comment 7 Chris Dumez 2014-10-03 11:05:16 PDT
(In reply to comment #6)
> Just a couple of comments:
> 1. If a Networking process crashes and the WebContext launches a new one to replace it, it will lose this state. To fix this, the WebContext needs to keep the flag and include it in the initialization parameters to the Networking process.
> 2. This should also be exposed as WKProcessPool SPI so that it can be used by clients of that API.

Thanks for the feedback, this is very helpful. I will address these issues soon.
Comment 8 Chris Dumez 2014-10-03 11:52:47 PDT
Created attachment 239218 [details]
Patch
Comment 9 Chris Dumez 2014-10-03 12:32:33 PDT
(In reply to comment #6)
> Just a couple of comments:
> 1. If a Networking process crashes and the WebContext launches a new one to replace it, it will lose this state. To fix this, the WebContext needs to keep the flag and include it in the initialization parameters to the Networking process.
> 2. This should also be exposed as WKProcessPool SPI so that it can be used by clients of that API.

All done in the latest patch iteration, thanks.
Comment 10 Chris Dumez 2014-10-06 12:34:46 PDT
Ping review?
Comment 11 mitz 2014-10-06 14:26:30 PDT
Comment on attachment 239218 [details]
Patch

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

> Source/WebKit2/ChangeLog:9
> +        a lot of cpu time serializing ProtectionSpace objects (in particular

CPU

> Source/WebKit2/ChangeLog:46
> +

You need to update the change log to include the WKProcessPool changes.
Comment 12 Chris Dumez 2014-10-06 14:30:57 PDT
Created attachment 239356 [details]
Patch
Comment 13 WebKit Commit Bot 2014-10-06 15:07:26 PDT
Comment on attachment 239356 [details]
Patch

Clearing flags on attachment: 239356

Committed r174369: <http://trac.webkit.org/changeset/174369>
Comment 14 WebKit Commit Bot 2014-10-06 15:07:32 PDT
All reviewed patches have been landed.  Closing bug.