Bug 135427 – Web Inspector: MessageDispatcher should not synchronously dispatch all backend messages

• Component: Web Inspector
• Status: RESOLVED FIXED

[Brian Burg]

patch coming

[Brian Burg]

Created attachment 235761 [details]
Patch

[Joseph Pecoraro]

Comment on attachment 235761 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Protocol/MessageDispatcher.js:50
> +        this._dispatchTimeout = setTimeout(this.dispatchNextQueuedMessageFromBackend.bind(this), 0);

We should avoid this.dispatchNextQueuedMessageFromBackend.bind every time this happens and have a pre-bound version of this that we can call. Or eliminate all uses of "this." and convert to "WebInspector." and we won't need a bound function.

[Brian Burg]

Created attachment 235763 [details]
example output 1

[Brian Burg]

Created attachment 235764 [details]
example output 2

the first attachment shows logspew when reloading cnn.com as the inspector is viewing resources.
the second is the same reload while looking at timeline overviews.

[Timothy Hatcher]

Comment on attachment 235761 [details]
Patch

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

> Source/WebInspectorUI/UserInterface/Protocol/InspectorBackend.js:176
> +            var processingDuration = Date.now() - processingStartTime;

performance.now()! You wil get better numbers.

> Source/WebInspectorUI/UserInterface/Protocol/InspectorBackend.js:223
> +        var processingDuration = Date.now() - processingStartTime;

Ditto.

> Source/WebInspectorUI/UserInterface/Protocol/MessageDispatcher.js:32
> +    var startTime = Date.now();

performance.now()

> Source/WebInspectorUI/UserInterface/Protocol/MessageDispatcher.js:36
> +    var i;
> +    for (i = 0; i < this.messagesToDispatch.length; ++i) {

I would put var i = 0; on the first line and leave the for loop initialize statement empty. Saves on var changes and will make the JIT happier.

> Source/WebInspectorUI/UserInterface/Protocol/MessageDispatcher.js:39
> +        if (Date.now() - startTime > timeLimitPerRunLoop)

performance.now()

[Brian Burg]

Comment on attachment 235761 [details]
Patch

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

>> Source/WebInspectorUI/UserInterface/Protocol/InspectorBackend.js:176
>> +            var processingDuration = Date.now() - processingStartTime;
> 
> performance.now()! You wil get better numbers.

Not enabled for Mac and Win ports, apparently. Tracked here: <https://bugs.webkit.org/show_bug.cgi?id=42434> and here: <https://bugs.webkit.org/show_bug.cgi?id=135467>

>> Source/WebInspectorUI/UserInterface/Protocol/MessageDispatcher.js:36
>> +    for (i = 0; i < this.messagesToDispatch.length; ++i) {
> 
> I would put var i = 0; on the first line and leave the for loop initialize statement empty. Saves on var changes and will make the JIT happier.

ok

>> Source/WebInspectorUI/UserInterface/Protocol/MessageDispatcher.js:50
>> +        this._dispatchTimeout = setTimeout(this.dispatchNextQueuedMessageFromBackend.bind(this), 0);
> 
> We should avoid this.dispatchNextQueuedMessageFromBackend.bind every time this happens and have a pre-bound version of this that we can call. Or eliminate all uses of "this." and convert to "WebInspector." and we won't need a bound function.

ok

[Radar WebKit Bug Importer]

<rdar://problem/17874168>

[Brian Burg]

Committed r171881: <http://trac.webkit.org/changeset/171881>