Network panel uses large amount of CSS that is mostly independent on the rest of the inspector. Moving a panel into iframe allows to decrease both number of HTML elements and number of CSS rules in a frame, thus largely reducing CSS rule matching time. On a primitive test (inspector on techcrunch.com), median time NetworkPanel.show() takes was reduced from 71ms to 9ms, first-time NetworkPanel show time went down from 1.6s to 1s, and for Scripts panel (untouched), median show time went from 88ms to 15ms.
(In reply to comment #0) > Network panel uses large amount of CSS that is mostly independent on the rest of the inspector. Moving a panel into iframe allows to decrease both number of HTML elements and number of CSS rules in a frame, thus largely reducing CSS rule matching time. > On a primitive test (inspector on techcrunch.com), median time NetworkPanel.show() takes was reduced from 71ms to 9ms, first-time NetworkPanel show time went down from 1.6s to 1s, and for Scripts panel (untouched), median show time went from 88ms to 15ms. 1.6s -> 1s for first show still sounds very slow. Do you know why?
(In reply to comment #1) > (In reply to comment #0) > > Network panel uses large amount of CSS that is mostly independent on the rest of the inspector. Moving a panel into iframe allows to decrease both number of HTML elements and number of CSS rules in a frame, thus largely reducing CSS rule matching time. > > On a primitive test (inspector on techcrunch.com), median time NetworkPanel.show() takes was reduced from 71ms to 9ms, first-time NetworkPanel show time went down from 1.6s to 1s, and for Scripts panel (untouched), median show time went from 88ms to 15ms. > > 1.6s -> 1s for first show still sounds very slow. Do you know why? 1s is for the first show of techcrunch's network log, empty case is much faster. I did not dig it yet, but my guess is that it's just populating timeline with network items.
> 1s is for the first show of techcrunch's network log, empty case is much faster. I think it is reasonable to require a 100ms feedback on this one, even if it ends up with deferred load. Btw, content of offscreen rows in the network panel is not rendered, only bounding boxes are. So it should not depend on the amount of resources much.
Created attachment 95897 [details] patch
(In reply to comment #3) > > 1s is for the first show of techcrunch's network log, empty case is much faster. > > I think it is reasonable to require a 100ms feedback on this one, even if it ends up with deferred load. Btw, content of offscreen rows in the network panel is not rendered, only bounding boxes are. So it should not depend on the amount of resources much. I'll have a look at this, but let's treat it as an independent issue.
Comment on attachment 95897 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=95897&action=review Overall looks good, couple of comments inline. > Source/WebCore/WebCore.vcproj/WebCore.vcproj:67676 > + RelativePath="..\inspector\front-end\inspectorCommon.js" inspectorCommon.css? > Source/WebCore/inspector/front-end/NetworkPanel.js:42 > + "textViewer.css", So we multiply the styles for editor. Is there a vision that converts views to iframes. > Source/WebCore/inspector/front-end/NetworkPanel.js:46 > + WebInspector.IFramePanel.call(this, "network", styles); Instead of a generic "iframe per panel" approach, I'd rather introduce "iframe-based views". The real thing we need here is a component model with the fine-grained APIs. Having said that, entire network panel can be a view. > Source/WebCore/inspector/front-end/Panel.js:493 > + this._iframeElement.contentDocument.head.appendChild(style); Are you fighting for the sync load here? It does not look pretty. > Source/WebCore/inspector/front-end/Panel.js:505 > + this._iframeElement = document.createElement("iframe"); I guess this should be done lazily in order not to regress the inspector open time. > Source/WebCore/inspector/front-end/Panel.js:522 > + var boundDispatchEvent = this._iframeElement.dispatchEvent.bind(this._iframeElement); Sometimes drag'n'drop events suffer from such dispatching. Please check whether it works for you.
One thing I noticed after applying the patch: inspector thinks it has no focus when network tab is selected.
Comment on attachment 95897 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=95897&action=review > Source/WebCore/inspector/front-end/NetworkPanel.js:43 > + "inspectorSyntaxHighlight.css", These guys are concatenated in the release mode. r- for this.
Created attachment 96109 [details] patch - Moved iframe logic into view, inherit IFramePanel from IFrameView - Added delayed creation of IFrameView - Fixed some bugs
(In reply to comment #6) > (From update of attachment 95897 [details]) > View in context: https://bugs.webkit.org/attachment.cgi? > > > inspectorCommon.css? Yep, thanks! > > > Source/WebCore/inspector/front-end/NetworkPanel.js:42 > > + "textViewer.css", > > So we multiply the styles for editor. Is there a vision that converts views to iframes. > > > Source/WebCore/inspector/front-end/NetworkPanel.js:46 > > + WebInspector.IFramePanel.call(this, "network", styles); > > Instead of a generic "iframe per panel" approach, I'd rather introduce "iframe-based views". The real thing we need here is a component model with the fine-grained APIs. Having said that, entire network panel can be a view. Added IFrameView. NetworkPanel still inherits is an IFramePanel, it looks like we borrow quite a bit from panels, and converting panels into views may be a bit messy. > > Source/WebCore/inspector/front-end/Panel.js:493 > > + this._iframeElement.contentDocument.head.appendChild(style); > > Are you fighting for the sync load here? It does not look pretty. Yes -- otherwise, we attempt to do things based on layout (e.g. table column sizing) before styles are loaded. > > Source/WebCore/inspector/front-end/Panel.js:505 > > + this._iframeElement = document.createElement("iframe"); > > I guess this should be done lazily in order not to regress the inspector open time. Done. > > > Source/WebCore/inspector/front-end/Panel.js:522 > > + var boundDispatchEvent = this._iframeElement.dispatchEvent.bind(this._iframeElement); > > Sometimes drag'n'drop events suffer from such dispatching. Please check whether it works for you. column resizing works, is there anything else to check?
(In reply to comment #7) > One thing I noticed after applying the patch: inspector thinks it has no focus when network tab is selected. Fixed.
(In reply to comment #8) > (From update of attachment 95897 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=95897&action=review > > > Source/WebCore/inspector/front-end/NetworkPanel.js:43 > > + "inspectorSyntaxHighlight.css", > > These guys are concatenated in the release mode. r- for this. It was actually working fine -- we now also have standalone copies of the relevant CSS files in the resources. As discussed, I'm leaving this mostly as is, though now I concatenate files in run-time before appending a style element. On my tests, we spend 2ms on XHR and 5ms on inserting the style into document (cumulative for all stylesheets used in the panel).
Comment on attachment 96109 [details] patch As we agreed offline, this is not ready for review.
Created attachment 100958 [details] Patch
Created attachment 101145 [details] patch - fixed handling of keyboard events
Created attachment 101158 [details] patch - fixed search - NetworkLogView.EventNames -> EventTypes
Comment on attachment 101158 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=101158&action=review > Source/WebCore/inspector/front-end/View.js:87 > +WebInspector.IFrameView = function(parent, stylesheets) Could you extract this into a separate file? > Source/WebCore/inspector/front-end/inspector.css:-2193 > -.data-grid { Could you move data grid into data grid instead? Also, could you concatenate these files?
Manually committed r91359: http://trac.webkit.org/changeset/91359