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.
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
Created attachment 329401 [details] [Image] Profile of Inspector during snapshot creation
<rdar://problem/36058755>
(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).
I think the changes made in <https://webkit.org/b/182995> and <https://webkit.org/b/185152> have resolved this bug.