Bug 160112 - REGRESSION (r202876): Web Inspector: Switching away from Console tab with a lot of messages is slow
Summary: REGRESSION (r202876): Web Inspector: Switching away from Console tab with a l...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Nikita Vasilyev
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-07-22 21:25 PDT by Nikita Vasilyev
Modified: 2016-07-23 00:18 PDT (History)
7 users (show)

See Also:


Attachments
[HTML] Reduction (384 bytes, text/html)
2016-07-22 21:25 PDT, Nikita Vasilyev
no flags Details
Patch (1.99 KB, patch)
2016-07-22 21:33 PDT, Nikita Vasilyev
joepeck: review+
joepeck: commit-queue+
Details | Formatted Diff | Diff
[Image] Before/After (179.38 KB, image/png)
2016-07-22 21:39 PDT, Nikita Vasilyev
no flags Details
Patch (2.22 KB, patch)
2016-07-22 23:03 PDT, Nikita Vasilyev
nvasilyev: review-
nvasilyev: commit-queue+
Details | Formatted Diff | Diff
Patch (2.24 KB, patch)
2016-07-22 23:29 PDT, Nikita Vasilyev
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nikita Vasilyev 2016-07-22 21:25:57 PDT
Created attachment 284396 [details]
[HTML] Reduction

This regressed in:
https://trac.webkit.org/changeset/202876/trunk/Source/WebInspectorUI/UserInterface/Views/TabBrowser.js

Steps:
1. Open attached reduction
2. Open Web Inspector
3. Open Console tab
4. Clear console
5. Reload the page
6. Click on Debugger tab

Expected:
Debugger tab opens in <0.5 seconds

Actual:
Web Inspector UI becomes unresponsive for 2-3 seconds before it opens Debugger tab.


rdar://problem/27431121
Comment 1 Nikita Vasilyev 2016-07-22 21:33:24 PDT
Created attachment 284397 [details]
Patch
Comment 2 Nikita Vasilyev 2016-07-22 21:39:23 PDT
Created attachment 284399 [details]
[Image] Before/After
Comment 3 Nikita Vasilyev 2016-07-22 21:43:38 PDT
With the patch applied, I was unable to reproduce bug 158069.
cq- because I don't yet understand the change that was made in that bug.
Comment 4 Nikita Vasilyev 2016-07-22 21:53:53 PDT
(In reply to comment #2)
> Created attachment 284399 [details]
> [Image] Before/After

Hm, I'm getting different results every time.
I'm still seeing 2-3x performance improvement on average.
Comment 5 Joseph Pecoraro 2016-07-22 22:48:38 PDT
Comment on attachment 284397 [details]
Patch

r=me, Tim's review comment at the time made this change unnecessary anyways, so win win!
Comment 6 Joseph Pecoraro 2016-07-22 22:50:38 PDT
Comment on attachment 284397 [details]
Patch

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

> Source/WebInspectorUI/ChangeLog:11
> +        (WebInspector.TabBrowser.prototype._tabBarItemSelected):
> +        Showing NavigationSidebar before removing previous TabContentView can be very slow when
> +        TabContentView is ConsoleTabContentView with a lot of messages.

This comment just says "can be slow" but doesn't state the reason WHY it is slow.

In this case, I believe the WHY was because it caused extra forced layouts because of the order of operations (show sidebar, change content view, show sidebar) instead of (change content view, show sidebar, show sidebar).
Comment 7 Nikita Vasilyev 2016-07-22 23:03:30 PDT
Created attachment 284403 [details]
Patch
Comment 8 Joseph Pecoraro 2016-07-22 23:15:55 PDT
Comment on attachment 284403 [details]
Patch

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

> Source/WebInspectorUI/ChangeLog:9
> +        TabContentView is ConsoleTabContentView with a lot of messages because of the order of operations:

Again "because of the order of operations" doesn't explain what is slow. It is because this different order of operations triggered MORE forced layouts.
Comment 9 Nikita Vasilyev 2016-07-22 23:29:09 PDT
Created attachment 284404 [details]
Patch
Comment 10 WebKit Commit Bot 2016-07-23 00:18:37 PDT
Comment on attachment 284404 [details]
Patch

Clearing flags on attachment: 284404

Committed r203636: <http://trac.webkit.org/changeset/203636>
Comment 11 WebKit Commit Bot 2016-07-23 00:18:42 PDT
All reviewed patches have been landed.  Closing bug.