RESOLVED FIXED 137367
[Mac] We are spending too much time serializing ProtectionSpace objects
https://bugs.webkit.org/show_bug.cgi?id=137367
Summary [Mac] We are spending too much time serializing ProtectionSpace objects
Chris Dumez
Reported 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.
Attachments
WIP Patch (7.39 KB, patch)
2014-10-02 17:06 PDT, Chris Dumez
no flags
WIP Patch (7.09 KB, patch)
2014-10-02 17:10 PDT, Chris Dumez
no flags
Patch (9.89 KB, patch)
2014-10-03 10:37 PDT, Chris Dumez
no flags
Patch (14.66 KB, patch)
2014-10-03 11:52 PDT, Chris Dumez
no flags
Patch (15.25 KB, patch)
2014-10-06 14:30 PDT, Chris Dumez
no flags
Chris Dumez
Comment 1 2014-10-02 16:37:36 PDT
I was thinking "setShouldHandleHTTPSServerTrustEvaluationAtNetworkLevel(bool)" for the (private) setting. Any thoughts?
Chris Dumez
Comment 2 2014-10-02 17:06:58 PDT
Created attachment 239158 [details] WIP Patch
Chris Dumez
Comment 3 2014-10-02 17:09:17 PDT
Chris Dumez
Comment 4 2014-10-02 17:10:56 PDT
Created attachment 239159 [details] WIP Patch
Chris Dumez
Comment 5 2014-10-03 10:37:35 PDT
mitz
Comment 6 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.
Chris Dumez
Comment 7 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.
Chris Dumez
Comment 8 2014-10-03 11:52:47 PDT
Chris Dumez
Comment 9 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.
Chris Dumez
Comment 10 2014-10-06 12:34:46 PDT
Ping review?
mitz
Comment 11 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.
Chris Dumez
Comment 12 2014-10-06 14:30:57 PDT
WebKit Commit Bot
Comment 13 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>
WebKit Commit Bot
Comment 14 2014-10-06 15:07:32 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.