Bug 157535 – Web Inspector: Avoid unnecessary timeout identifier churn in TimelineManager

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

[Joseph Pecoraro]

* SUMMARY
Avoid unnecessary timeout identifier churn in TimelineManager.

The _deadTimeTimeout can get churned thousands of times (4000+ on theverge.com) because a bunch of records are added at once.

Reduce the churn on the timeout identifier (<100), which also slightly reduces the amount of time spent in _resetAutoRecordingDeadTimeTimeout.

[Joseph Pecoraro]

Created attachment 278531 [details]
[PATCH] Proposed Fix

[Joseph Pecoraro]

Comment on attachment 278531 [details]
[PATCH] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Controllers/TimelineManager.js:744
> +        let now = Date.now();
> +        if (now <= this._lastDeadTimeTickle + 10)
> +            return;
> +
> +        this._lastDeadTimeTickle = now;

We could make the this._lastDeadTimeTickle = now + 10. And eliminate the "+ 10", thereby eliminating the add every check. But it felt less clear. Still the time spent in this was only ~5ms on a long recording on theverge, so not very important.

[Matt Baker]

Comment on attachment 278531 [details]
[PATCH] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Controllers/TimelineManager.js:741
> +        if (now <= this._lastDeadTimeTickle + 10)

Magic number: a comment at the end of the line or a const on the line above would help clarify the 10.

[Timothy Hatcher]

Comment on attachment 278531 [details]
[PATCH] Proposed Fix

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

> Source/WebInspectorUI/UserInterface/Controllers/TimelineManager.js:740
> +        let now = Date.now();

performance.now?

>> Source/WebInspectorUI/UserInterface/Controllers/TimelineManager.js:744
>> +        this._lastDeadTimeTickle = now;
> 
> We could make the this._lastDeadTimeTickle = now + 10. And eliminate the "+ 10", thereby eliminating the add every check. But it felt less clear. Still the time spent in this was only ~5ms on a long recording on theverge, so not very important.

I think putting the + 10 here makes more sense.

[Joseph Pecoraro]

Created attachment 278555 [details]
[PATCH] For Landing

[WebKit Commit Bot]

Comment on attachment 278555 [details]
[PATCH] For Landing

Clearing flags on attachment: 278555

Committed r200661: <http://trac.webkit.org/changeset/200661>