Bug 180838 - Web Inspector: Canvas tab: improve UI responsiveness when creating recording snapshots
Summary: Web Inspector: Canvas tab: improve UI responsiveness when creating recording ...
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: Matt Baker
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2017-12-14 13:59 PST by Matt Baker
Modified: 2018-08-23 16:18 PDT (History)
3 users (show)

See Also:


Attachments
[Recording] Large (~2MB) recording with ~5000 actions (761.21 KB, application/json)
2017-12-14 13:59 PST, Matt Baker
no flags Details
[Image] Profile of Inspector during snapshot creation (588.52 KB, image/png)
2017-12-14 14:00 PST, Matt Baker
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Baker 2017-12-14 13:59:27 PST
Created attachment 329400 [details]
[Recording] Large (~2MB) recording with ~5000 actions

Summary:
Improve UI responsiveness when creating recording snapshots. A significant amount of time is spent getting the canvas content whenever RecordingAction.apply is called, which can happen many times during the creation of a single snapshot.

Moving application of actions to a YieldableTask should help mitigate UI lockup, and allow the task to be cancelled/restarted if a new snapshot is requested before the current one completes rendering.
Comment 1 Matt Baker 2017-12-14 14:00:21 PST
Steps to Reproduce:

1. Load attached recording
2. Open recording view
3. Slide should be at action 0 of 5k
4. Click somewhere around the middle of the slider
  => Takes multiple seconds to load snapshot
Comment 2 Matt Baker 2017-12-14 14:00:45 PST
Created attachment 329401 [details]
[Image] Profile of Inspector during snapshot creation
Comment 3 Radar WebKit Bug Importer 2017-12-14 14:38:01 PST
<rdar://problem/36058755>
Comment 4 Devin Rousso 2017-12-14 16:31:23 PST
(In reply to Matt Baker from comment #0)
> Improve UI responsiveness when creating recording snapshots. A significant
> amount of time is spent getting the canvas content whenever
> RecordingAction.apply is called, which can happen many times during the
> creation of a single snapshot.
So the issue here isn't really the creation of the snapshot, but more-so the way we calculate/determine whether or not a visual action had any effect.  The `getContent` call(s) within `RecordingAction.prototype.apply` will only happen if that RecordingAction is visual and has not yet checked whether it had an effect.  As such, this is really only an issue the first time each action is applied.  I think there are two ways to solve this:
 1) Check every visual action up front, and create all snapshots (every ~5000 actions) before we display the RecordingContentView, possibly with some sort of progress indicator
 2) Only perform this check when a visual action is directly selected in the UI, meaning that it was either scrubbed to from the range input or had it's TreeElement clicked

I personally think that (1) is a better solution, because (2) is very "intermittent" in that a RecordingAction could appear to have an effect and then suddenly a warning icon appears once it was clicked.  This may be reasonable, considering we are "replaying" the recording, but I wouldn't really expect that.  Also, we already have a pretty intense workload when loading a recording anyways, so adding a bit more too it could be considered "reasonable".

> Moving application of actions to a YieldableTask should help mitigate UI
> lockup, and allow the task to be cancelled/restarted if a new snapshot is
> requested before the current one completes rendering.
I don't see how moving this to a YieldableTask would help.  There really is no way to "cancel" RecordingAction.prototype.apply, as we need to properly save/restore the context after the group of actions is applied.  I'm sure there may be a way to do this nicely with some logic, but I think we are better off taking one of the approaches above than trying to split the application of the actions to multiple frames (doing this might introduce flashes of intermediate canvas content depending on how expensive the operation is, which I don't think is desirable).
Comment 5 Devin Rousso 2018-08-23 16:18:01 PDT
I think the changes made in <https://webkit.org/b/182995> and <https://webkit.org/b/185152> have resolved this bug.