Bug 170525

Summary: Web Inspector: Only Capture Extra Network Load Metrics when there is a Web Inspector Frontend
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: Web InspectorAssignee: Joseph Pecoraro <joepeck>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, beidson, buildbot, cdumez, cgarcia, commit-queue, dbates, inspector-bugzilla-changes, japhet, joepeck, keith_miller, koivisto, mark.lam, msaboff, saam, timothy, youennf
Priority: P2    
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
See Also: https://bugs.webkit.org/show_bug.cgi?id=187607
Bug Depends on: 16531    
Bug Blocks:    
Attachments:
Description Flags
[PATCH] Proposed Fix
none
[PATCH] For Bots (All Parts)
none
[PATCH] For Bots (All Parts)
none
[PATCH] For Bots (All Parts)
none
[PATCH] For Bots (All Parts)
buildbot: commit-queue-
Archive of layout-test-results from ews123 for ios-simulator-wk2
none
[PATCH] Proposed Fix none

Joseph Pecoraro
Reported 2017-04-05 16:00:08 PDT
Summary: Only Capture Extra Network Load Metrics when there is a Web Inspector Frontend To limit any potential performance impact from making another copy of all HTTP Headers, only do this work in the NetworkProcess if the WebProcess has a Web Inspector frontend open. We can extend this to a few other Metrics properties as well.
Attachments
[PATCH] Proposed Fix (21.73 KB, patch)
2017-04-05 16:05 PDT, Joseph Pecoraro
no flags
[PATCH] For Bots (All Parts) (44.25 KB, patch)
2017-04-05 16:06 PDT, Joseph Pecoraro
no flags
[PATCH] For Bots (All Parts) (44.21 KB, patch)
2017-04-05 16:25 PDT, Joseph Pecoraro
no flags
[PATCH] For Bots (All Parts) (44.21 KB, patch)
2017-04-05 16:49 PDT, Joseph Pecoraro
no flags
[PATCH] For Bots (All Parts) (44.24 KB, patch)
2017-04-05 17:36 PDT, Joseph Pecoraro
buildbot: commit-queue-
Archive of layout-test-results from ews123 for ios-simulator-wk2 (37.35 MB, application/zip)
2017-04-05 19:28 PDT, Build Bot
no flags
[PATCH] Proposed Fix (21.81 KB, patch)
2017-04-06 15:03 PDT, Joseph Pecoraro
no flags
Joseph Pecoraro
Comment 1 2017-04-05 16:05:54 PDT
Created attachment 306328 [details] [PATCH] Proposed Fix
Joseph Pecoraro
Comment 2 2017-04-05 16:06:39 PDT
Created attachment 306329 [details] [PATCH] For Bots (All Parts)
Joseph Pecoraro
Comment 3 2017-04-05 16:15:33 PDT
This prevents copying of headers in NetworkProcess, Serializing across to the WebProcess, and potentially copying of the HTTPHeaderMap in the WebProcess. It also avoids setting a few other properties only needed for Web Inspector (and a few others are coming).
Joseph Pecoraro
Comment 4 2017-04-05 16:25:25 PDT
Comment on attachment 306328 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=306328&action=review > Source/WebKit2/NetworkProcess/NetworkLoad.h:73 > + bool shouldCaptureExtraNetworkLoadMetrics() const final; This needs to be inside of a USE(NETWORK_SESSION) block. Fixed locally.
Joseph Pecoraro
Comment 5 2017-04-05 16:25:42 PDT
Created attachment 306333 [details] [PATCH] For Bots (All Parts)
Joseph Pecoraro
Comment 6 2017-04-05 16:49:47 PDT
Created attachment 306337 [details] [PATCH] For Bots (All Parts) A few more USE(NETWORK_SESSION)
Joseph Pecoraro
Comment 7 2017-04-05 17:36:32 PDT
Created attachment 306342 [details] [PATCH] For Bots (All Parts)
Build Bot
Comment 8 2017-04-05 19:28:31 PDT
Comment on attachment 306342 [details] [PATCH] For Bots (All Parts) Attachment 306342 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: http://webkit-queues.webkit.org/results/3482094 New failing tests: http/tests/cache/disk-cache/speculative-validation/http-auth.html
Build Bot
Comment 9 2017-04-05 19:28:34 PDT
Created attachment 306356 [details] Archive of layout-test-results from ews123 for ios-simulator-wk2 The attached test failures were seen while running run-webkit-tests on the ios-sim-ews. Bot: ews123 Port: ios-simulator-wk2 Platform: Mac OS X 10.11.6
Joseph Pecoraro
Comment 10 2017-04-06 13:35:34 PDT
Ping reviewers.
youenn fablet
Comment 11 2017-04-06 14:06:32 PDT
Comment on attachment 306328 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=306328&action=review > Source/WebCore/inspector/InspectorInstrumentation.h:1357 > + if (s_frontendCounter == 1) one liner? > Source/WebCore/inspector/InspectorInstrumentation.h:1365 > + if (!s_frontendCounter) one liner? > Source/WebKit2/NetworkProcess/cocoa/NetworkSessionCocoa.mm:315 > + requestHeaders.set(String(name), String(value)); Might want to consider sending only the modified headers if this can be easily done.
youenn fablet
Comment 12 2017-04-06 14:06:32 PDT
Comment on attachment 306328 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=306328&action=review > Source/WebCore/inspector/InspectorInstrumentation.h:1357 > + if (s_frontendCounter == 1) one liner? > Source/WebCore/inspector/InspectorInstrumentation.h:1365 > + if (!s_frontendCounter) one liner? > Source/WebKit2/NetworkProcess/cocoa/NetworkSessionCocoa.mm:315 > + requestHeaders.set(String(name), String(value)); Might want to consider sending only the modified headers if this can be easily done.
youenn fablet
Comment 13 2017-04-06 14:06:51 PDT
(In reply to Joseph Pecoraro from comment #10) > Ping reviewers. Can you rebase it?
Joseph Pecoraro
Comment 14 2017-04-06 14:18:46 PDT
(In reply to youenn fablet from comment #13) > (In reply to Joseph Pecoraro from comment #10) > > Ping reviewers. > > Can you rebase it? It depends on a patch that hasn't landed yet (hence the "All Parts" patch). But I'll go ahead and land the earlier part and put up a new patch.
Joseph Pecoraro
Comment 15 2017-04-06 14:20:38 PDT
> > Source/WebCore/inspector/InspectorInstrumentation.h:1357 > > + if (s_frontendCounter == 1) > > one liner? > > > Source/WebCore/inspector/InspectorInstrumentation.h:1365 > > + if (!s_frontendCounter) > > one liner? The compiler should optimize this, I aimed for the code to have maximum readability / clarity.
Joseph Pecoraro
Comment 16 2017-04-06 15:03:38 PDT
Created attachment 306431 [details] [PATCH] Proposed Fix
WebKit Commit Bot
Comment 17 2017-04-06 15:58:11 PDT
Comment on attachment 306431 [details] [PATCH] Proposed Fix Clearing flags on attachment: 306431 Committed r215065: <http://trac.webkit.org/changeset/215065>
WebKit Commit Bot
Comment 18 2017-04-06 15:58:13 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.