RESOLVED FIXED 177468
Web Inspector: Improve Table scrolling performance
https://bugs.webkit.org/show_bug.cgi?id=177468
Summary Web Inspector: Improve Table scrolling performance
Joseph Pecoraro
Reported 2017-09-25 15:04:25 PDT
Improve Table scrolling performance. I was seeing a lot of work while scrolling recalculating styles by checking the width/height of the table. During scrolling these values won't change. Only a resize will change these values. So we can cache them and avoid recalculating them unless we've actually resized.
Attachments
[PATCH] Proposed Fix (10.61 KB, patch)
2017-09-25 15:14 PDT, Joseph Pecoraro
bburg: review+
Joseph Pecoraro
Comment 1 2017-09-25 15:14:03 PDT
Created attachment 321752 [details] [PATCH] Proposed Fix
Joseph Pecoraro
Comment 2 2017-09-25 18:37:50 PDT
Comment on attachment 321752 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=321752&action=review > Source/WebInspectorUI/UserInterface/Views/Table.js:908 > + // Completely remove all rows and add new ones. I believe this is the only remaining work that shows in profiles. However the issue is less with scrolling and more with -reloadData needing to throw out old rows and create all new ones. If this actually ends up being a problem we can try using a pool of elements or an approach that doesn't remove and reinsert elements every time.
Blaze Burg
Comment 3 2017-09-26 11:01:07 PDT
Comment on attachment 321752 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=321752&action=review r=me > Source/WebInspectorUI/UserInterface/Views/Table.js:671 > + let availableHeight = this._cachedHeight; I'm not sure about usage of these locals. Can you get rid of them, or uses them instead of the member after this point? > Source/WebInspectorUI/UserInterface/Views/Table.js:968 > + if (row.__widthGeneration !== this._widthsGeneration && row !== this._fillerRow) { In the interest of reducing indent, can you make this an early continue if there's nothing to do? > Source/WebInspectorUI/UserInterface/Views/Table.js:973 > + row.__widthGeneration = this._widthsGeneration; It bothers me that widthGeneration and widthsGeneration are spelled differently. Do they need to differ?
Joseph Pecoraro
Comment 4 2017-09-26 11:10:27 PDT
Comment on attachment 321752 [details] [PATCH] Proposed Fix View in context: https://bugs.webkit.org/attachment.cgi?id=321752&action=review >> Source/WebInspectorUI/UserInterface/Views/Table.js:973 >> + row.__widthGeneration = this._widthsGeneration; > > It bothers me that widthGeneration and widthsGeneration are spelled differently. Do they need to differ? Haha, this was almost certainly a find and replace mistake on my part. I'll clean this up. Thanks!
Joseph Pecoraro
Comment 5 2017-09-26 11:23:09 PDT
Radar WebKit Bug Importer
Comment 6 2017-09-27 12:16:23 PDT
Note You need to log in before you can comment on or make changes to this bug.