WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
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
<
https://trac.webkit.org/changeset/222509/webkit
>
Radar WebKit Bug Importer
Comment 6
2017-09-27 12:16:23 PDT
<
rdar://problem/34692907
>
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug