Bug 140550 – Web Inspector: Disable *Annotators on minified files that aren't pretty printed

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

[Saam Barati]

The performance of the BasicBlockAnnotator and the TypeTokenAnnotator on files with really long lines is awful.
It's also not of much use viewing this profiling information when a minified file isn't pretty printed, so we
should just disable the profilers when this happens.

[Radar WebKit Bug Importer]

<rdar://problem/19501493>

[Saam Barati]

Created attachment 245160 [details]
patch

[Build Bot]

Comment on attachment 245160 [details]
patch

Attachment 245160 [details] did not pass mac-ews (mac):
Output: http://webkit-queues.appspot.com/results/5927189312700416

New failing tests:
http/tests/appcache/404-resource-with-slow-main-resource.php

[Build Bot]

Created attachment 245165 [details]
Archive of layout-test-results from ews102 for mac-mavericks

The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews102  Port: mac-mavericks  Platform: Mac OS X 10.9.5

[Timothy Hatcher]

Comment on attachment 245160 [details]
patch

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

> Source/WebInspectorUI/ChangeLog:20
> +        When SourceCodeTextEditor speculates that a file is minified it 
> +        sets a flag on itself that the Annotators will notice when pretty 
> +        printing is disabled. If we think a file is minified, we will
> +        disable the Annotators when turning off pretty printing because
> +        the Annotators have bad performance while viewing truly minified
> +        files.
> +        
> +        Sometimes, though, a file won't be minified even though we speculate
> +        that it is. Even though we may speculate incorrectly, if Type
> +        Profiling is enabled while viewing a file that we speculate as 
> +        minified in its minified form, we first require pretty printing
> +        the file before displaying the Annotators to prevent truly
> +        bad performance.

Will the navigation bar item for type profiling be disabled when pretty printing is required? Or should clicking the type profiling item force pretty printing to be enabled as well?

[Saam Barati]

(In reply to comment #5)
> Comment on attachment 245160 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=245160&action=review
> 
> > Source/WebInspectorUI/ChangeLog:20
> > +        When SourceCodeTextEditor speculates that a file is minified it 
> > +        sets a flag on itself that the Annotators will notice when pretty 
> > +        printing is disabled. If we think a file is minified, we will
> > +        disable the Annotators when turning off pretty printing because
> > +        the Annotators have bad performance while viewing truly minified
> > +        files.
> > +        
> > +        Sometimes, though, a file won't be minified even though we speculate
> > +        that it is. Even though we may speculate incorrectly, if Type
> > +        Profiling is enabled while viewing a file that we speculate as 
> > +        minified in its minified form, we first require pretty printing
> > +        the file before displaying the Annotators to prevent truly
> > +        bad performance.
> 
> Will the navigation bar item for type profiling be disabled when pretty
> printing is required? Or should clicking the type profiling item force
> pretty printing to be enabled as well?

Sorry, this ChangeLog entry isn't clear. I'll fix it up in the patch I commit.

This patch enforces these two behaviors:
1. Always enforce a file to be pretty printed if we think it's minified while viewing type annotations. So, while viewing a minified file, if you press the type profiling button, the pretty printer will first reformat the file, then the type profiler will insert type annotations.

2. If the type profiler is enabled and we're viewing a pretty printed file that we think originally was minified, and the pretty printer is disabled via pressing the button, then the type profiler will also be disabled.

Originally, I had just behavior 2 and not behavior 1, and allowed type annotations to be inserted on a minified file while viewing it in its minified form. This provided a bad user experience for both performance and visual reasons, which is the reason for creating behavior 1.

[Saam Barati]

landed in:
http://trac.webkit.org/changeset/179016

[Timothy Hatcher]

Comment on attachment 245160 [details]
patch

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

>>> Source/WebInspectorUI/ChangeLog:20
>>> +        bad performance.
>> 
>> Will the navigation bar item for type profiling be disabled when pretty printing is required? Or should clicking the type profiling item force pretty printing to be enabled as well?
> 
> Sorry, this ChangeLog entry isn't clear. I'll fix it up in the patch I commit.
> 
> This patch enforces these two behaviors:
> 1. Always enforce a file to be pretty printed if we think it's minified while viewing type annotations. So, while viewing a minified file, if you press the type profiling button, the pretty printer will first reformat the file, then the type profiler will insert type annotations.
> 
> 2. If the type profiler is enabled and we're viewing a pretty printed file that we think originally was minified, and the pretty printer is disabled via pressing the button, then the type profiler will also be disabled.
> 
> Originally, I had just behavior 2 and not behavior 1, and allowed type annotations to be inserted on a minified file while viewing it in its minified form. This provided a bad user experience for both performance and visual reasons, which is the reason for creating behavior 1.

Makes sense. Just wanted to make sure. That is exactly what I expect.