Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Web Inspector: Search tab should lazily insert/remove its navigation sidebar tree items during scrolling #2175

Conversation

patrickangle
Copy link
Contributor

@patrickangle patrickangle commented Jul 7, 2022

0ccc6a9

Web Inspector: Search tab should lazily insert/remove its navigation sidebar tree items during scrolling
https://bugs.webkit.org/show_bug.cgi?id=242419
rdar://problem/96601304

Reviewed by Devin Rousso.

Improve performance of the Search tab by virtualizing the navigation tree outline, which allows us to reduce the number
of DOM elements that are attached to the DOM at any given point.

This accounts for about 50% of the performance improvements that have taken searching for the letter `a` on webkit.org
from ~13 seconds to under a second (using manual stopwatch testing across 5 tests before and after). The other 50% of
the improvement is from bug 242450 (Web Inspector: Lazily create SourceCodeTextRange for search result objects). Note
that very large sites still pose a problem, but instrumenting search performance has been tricky because they lock up
and never return performance/timing data.

* Source/WebInspectorUI/UserInterface/Views/SearchSidebarPanel.js:
(WI.SearchSidebarPanel):

Canonical link: https://commits.webkit.org/252279@main

@patrickangle patrickangle self-assigned this Jul 7, 2022
@patrickangle patrickangle added Web Inspector Bugs related to the WebKit Web Inspector. WebKit Nightly Build labels Jul 7, 2022
Copy link
Member

@dcrousso dcrousso left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you elaborate more what you mean by

Note that currently we only virtualize the top-level items (in this case a resource).

I'm pretty sure WI.TreeOutline.prototype.registerScrollVirtualizer knows how to handle nested WI.TreeElement 🤔

@@ -55,6 +55,7 @@ WI.SearchSidebarPanel = class SearchSidebarPanel extends WI.NavigationSidebarPan

WI.Frame.addEventListener(WI.Frame.Event.MainResourceDidChange, this._mainResourceDidChange, this);

this.contentTreeOutline.registerScrollVirtualizer(this.contentView.element, 20);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we completely sure that every tree item will have exactly 20px height? This entire system only works if that is the case 😅

Style: const treeItemHeight = 20;

@webkit-early-warning-system webkit-early-warning-system added the merging-blocked Applied to prevent a change from being merged label Jul 7, 2022
@patrickangle patrickangle removed the merging-blocked Applied to prevent a change from being merged label Jul 7, 2022
@patrickangle patrickangle force-pushed the eng/b242419-virtualized-search-tree-outline branch 2 times, most recently from cc207bf to bd0e1de Compare July 7, 2022 18:42
@patrickangle patrickangle added the merge-queue Applied to send a pull request to merge-queue label Jul 8, 2022
…sidebar tree items during scrolling

https://bugs.webkit.org/show_bug.cgi?id=242419
rdar://problem/96601304

Reviewed by Devin Rousso.

Improve performance of the Search tab by virtualizing the navigation tree outline, which allows us to reduce the number
of DOM elements that are attached to the DOM at any given point.

This accounts for about 50% of the performance improvements that have taken searching for the letter `a` on webkit.org
from ~13 seconds to under a second (using manual stopwatch testing across 5 tests before and after). The other 50% of
the improvement is from bug 242450 (Web Inspector: Lazily create SourceCodeTextRange for search result objects). Note
that very large sites still pose a problem, but instrumenting search performance has been tricky because they lock up
and never return performance/timing data.

* Source/WebInspectorUI/UserInterface/Views/SearchSidebarPanel.js:
(WI.SearchSidebarPanel):

Canonical link: https://commits.webkit.org/252279@main
@webkit-early-warning-system webkit-early-warning-system force-pushed the eng/b242419-virtualized-search-tree-outline branch from bd0e1de to 0ccc6a9 Compare July 8, 2022 17:00
@webkit-commit-queue
Copy link
Collaborator

Committed 252279@main (0ccc6a9): https://commits.webkit.org/252279@main

Reviewed commits have been landed. Closing PR #2175 and removing active labels.

@webkit-early-warning-system webkit-early-warning-system merged commit 0ccc6a9 into WebKit:main Jul 8, 2022
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jul 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Web Inspector Bugs related to the WebKit Web Inspector.
Projects
None yet
4 participants