Bug 177468 – Web Inspector: Improve Table scrolling performance

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector

[Joseph Pecoraro]

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.

[Joseph Pecoraro]

Created attachment 321752 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

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 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 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]

<https://trac.webkit.org/changeset/222509/webkit>

[Radar WebKit Bug Importer]

<rdar://problem/34692907>