Bug 95731 – Web Inspector: Timeline: cache "filteredRecords" for better scrolling performance.

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Inspector (Deprecated)

Now scrolling is sluggish on large datasets.
Performance degrades, because each refresh causes DFS on all recorded items.

Solution: caching/invalidation of DFS (filtering) results.

Created attachment 162005 [details]
Patch

[Andrey Kosyakov]

Comment on attachment 162005 [details]
Patch

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

> Source/WebCore/inspector/front-end/TimelinePanel.js:677
> +    _scheduleRefresh: function(preserveBoundaries, invalidateFilteredRecords)
>      {
> +        if (invalidateFilteredRecords)
> +            this._presentationModel.invalidateFilteredRecords();

Rather than passing additional bool to scheduleRefresh, can we instead perform invalidation on the call site?
You may want to extract this to a method like invalidateVisibleRecordsAndRefresh()

Created attachment 162183 [details]
Patch

[Andrey Kosyakov]

Comment on attachment 162183 [details]
Patch

LGTM

[Yury Semikhatsky]

Comment on attachment 162183 [details]
Patch

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

> Source/WebCore/inspector/front-end/TimelinePanel.js:638
> +        this._automaticallySizeWindow = this._automaticallySizeWindow || !preserveAutomaticallySizeWindow;

Could you explain this part?

[Andrey Kosyakov]

Comment on attachment 162183 [details]
Patch

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

>> Source/WebCore/inspector/front-end/TimelinePanel.js:638
>> +        this._automaticallySizeWindow = this._automaticallySizeWindow || !preserveAutomaticallySizeWindow;
> 
> Could you explain this part?

@eustas: can we instead unconditionally set this to false in repopulate? If we needed automatic resize, this should have happened by now and we would just need to preserve the window?

[Pavel Feldman]

As with any optimization that makes the code more complex, please provide some metrics for most common scenarios that prove speed ups. Why do we dfs at the first place? Should we bisect records in case of no glueing? Can we bisect with gliding? Caching has no real benefit, it will only eat memory here imo.

(In reply to comment #7)
> As with any optimization that makes the code more complex, please provide some metrics for most common scenarios that prove speed ups. Why do we dfs at the first place? Should we bisect records in case of no glueing? Can we bisect with gliding? Caching has no real benefit, it will only eat memory here imo.

In this case caching provides almost 10x performance boost (90ms -> 9ms on large dataset).

DFS is used, because visibility of child records is independent of visibility of parent records.

Of course "filteredRecords" could/would/should be optimized, but this patch is the first step.

With this patch scrolling becomes much-much smoother.

[Pavel Feldman]

OK, patch concept lgtm. But in reality instead of scrolling users will adjust window. We should bisect time instead of linear searching. That would fix all scenarios.

[Andrey Kosyakov]

(In reply to comment #9)
> OK, patch concept lgtm. But in reality instead of scrolling users will adjust window. We should bisect time instead of linear searching. That would fix all scenarios.

Why I agree we should bisect for window selection, it doesn't cover all the scenarios -- we also have filtering on type and (coming up) search. I don't think caching really "eats memory" here -- re-allocating the same array each time we scroll/resize actually does more harm, as we thrash the GC.

Created attachment 162479 [details]
Patch

[Andrey Kosyakov]

Comment on attachment 162479 [details]
Patch

LGTM

[WebKit Review Bot]

Comment on attachment 162479 [details]
Patch

Clearing flags on attachment: 162479

Committed r127746: <http://trac.webkit.org/changeset/127746>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.