Bug 157664 – Web Inspector: DataGrid _updateVisibleRows dominates profiles of timeline recordings when data grid (Overview or TimelineDataGrids) is showing

• Component: Web Inspector
• Status: RESOLVED FIXED

[Joseph Pecoraro]

* SUMMARY
DataGrid _updateVisibleRows dominates profiles of timeline recordings when data grid (Overview or TimelineDataGrids) is showing.

Profiling Web Inspector while recording a timeline with a DataGrid showing absolutely kills performance. Hiding DataGrids (e.g. Scripts and Events / ProfileView) Overview graphs are almost flawless!

* STEPS TO REPRODUCE
1. Inspect theverge.com
2. Show Timelines Tab
3. Show Network + Layout + Scripts timelines
4. Select Layout and Rendering Timeline to show DataGrid
5. Reload
  => Lots of Forced layouts dominating time in DataGrid _updateVisibleRows

[Radar WebKit Bug Importer]

<rdar://problem/26262219>

[Timothy Hatcher]

Created attachment 278942 [details]
Patch

[Build Bot]

Comment on attachment 278942 [details]
Patch

Attachment 278942 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.webkit.org/results/1321578

New failing tests:
imported/blink/storage/indexeddb/blob-basics-metadata.html

[Build Bot]

Created attachment 278943 [details]
Archive of layout-test-results from ews103 for mac-yosemite

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103  Port: mac-yosemite  Platform: Mac OS X 10.10.5

[Joseph Pecoraro]

Comment on attachment 278942 [details]
Patch

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

r=me

> Source/WebInspectorUI/UserInterface/Views/DataGrid.js:804
>          if (layoutReason == WebInspector.View.LayoutReason.Resize || firstUpdate) {

Nit: You could drive by fix the == to ===

> Source/WebInspectorUI/UserInterface/Views/DataGrid.js:1010
> +        if (isNaN(this._cachedScrollHeight))
> +            this._cachedScrollHeight = this._scrollContainerElement.offsetHeight;

Should we rename this offsetHeight given it is getting the offsetHeight instead of scrollHeight?

[Blaze Burg]

Comment on attachment 278942 [details]
Patch

r=me also

[Timothy Hatcher]

https://trac.webkit.org/r200949