From aec1ec3abcbd259ede0ce3dcb2608975a69b1db3 Mon Sep 17 00:00:00 2001 From: Mingjing Zhang Date: Mon, 6 Sep 2021 19:58:03 -0400 Subject: [PATCH] Align frames in the Frames timeline with BeginFrame events This CL improves the visualization of the Frames timeline such that frames are now demarcated with BeginFrame events instead of DrawFrame and DroppedFrame events. After this change, all visualized frames (both presented and dropped) are VSync-aligned and their durations are always multiples of 1/refresh rate. This new Frames timeline behaviour requires updates of some web tests, which were temporarily disabled by https://crrev.com/c/3197870 to allow the current CL to land. Design doc: https://docs.google.com/document/d/1XjOp05FoShB7bAGoovD4bCBL8L4E8vsEqsPfBKO8ko0/edit?usp=sharing Bug: 1192637 Change-Id: I98778bdc9e41502ed7705c5083c1dc681c27a7bb Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3083645 Commit-Queue: Mingjing Zhang Reviewed-by: Tim van der Lippe Reviewed-by: Paul Lewis --- .../timeline_model/TimelineFrameModel.ts | 135 ++++++++++++---- .../front_end/models/timeline_model/BUILD.gn | 5 +- .../TimelineFrameBeginFrameQueue_test.ts | 152 ++++++++++++++++++ 3 files changed, 263 insertions(+), 29 deletions(-) create mode 100644 test/unittests/front_end/models/timeline_model/TimelineFrameBeginFrameQueue_test.ts diff --git a/front_end/models/timeline_model/TimelineFrameModel.ts b/front_end/models/timeline_model/TimelineFrameModel.ts index f944d5c983..0d3c865b54 100644 --- a/front_end/models/timeline_model/TimelineFrameModel.ts +++ b/front_end/models/timeline_model/TimelineFrameModel.ts @@ -44,6 +44,7 @@ export class TimelineFrameModel { private frameById!: { [x: number]: TimelineFrame, }; + private beginFrameQueue!: TimelineFrameBeginFrameQueue; private minimumRecordTime!: number; private lastFrame!: TimelineFrame|null; private mainFrameCommitted!: boolean; @@ -115,6 +116,7 @@ export class TimelineFrameModel { this.minimumRecordTime = Infinity; this.frames = []; this.frameById = {}; + this.beginFrameQueue = new TimelineFrameBeginFrameQueue(); this.lastFrame = null; this.lastLayerTree = null; this.mainFrameCommitted = false; @@ -130,21 +132,28 @@ export class TimelineFrameModel { this.currentTaskTimeByCategory = {}; } - handleBeginFrame(startTime: number): void { + handleBeginFrame(startTime: number, seqId: number): void { if (!this.lastFrame) { this.startFrame(startTime); } this.lastBeginFrame = startTime; + + this.beginFrameQueue.addFrameIfNotExists(seqId, startTime, false); } - handleDroppedFrame(startTime: number): void { + handleDroppedFrame(startTime: number, seqId: number): void { if (!this.lastFrame) { this.startFrame(startTime); } - this.lastDroppedFrame = startTime; + + // This line handles the case where no BeginFrame event is issued for + // the dropped frame. In this situation, add a BeginFrame to the queue + // as if it actually occurred. + this.beginFrameQueue.addFrameIfNotExists(seqId, startTime, true); + this.beginFrameQueue.setDropped(seqId, true); } - handleDrawFrame(startTime: number): void { + handleDrawFrame(startTime: number, seqId: number): void { if (!this.lastFrame) { this.startFrame(startTime); return; @@ -158,20 +167,27 @@ export class TimelineFrameModel { (this.lastBeginFrame || this.lastNeedsBeginFrame); if (idleTimeEnd > this.lastFrame.startTime) { this.lastFrame.idle = true; - this.startFrame(idleTimeEnd); - if (this.framePendingActivation) { - this.commitPendingFrame(); - } this.lastBeginFrame = null; } this.lastNeedsBeginFrame = null; } - if (this.lastDroppedFrame) { - this.lastFrame.dropped = true; - this.startFrame(this.lastDroppedFrame); - this.lastDroppedFrame = null; + + const framesToVisualize = this.beginFrameQueue.processPendingBeginFramesOnDrawFrame(seqId); + + // Visualize the current frame and all pending frames before it. + for (const frame of framesToVisualize) { + const isLastFrameIdle = this.lastFrame.idle; + + // If |frame| is the first frame after an idle period, the CPU time + // will be logged ("committed") under |frame| if applicable. + this.startFrame(frame.startTime); + if (isLastFrameIdle && this.framePendingActivation) { + this.commitPendingFrame(); + } + if (frame.isDropped) { + this.lastFrame.dropped = true; + } } - this.startFrame(startTime); } this.mainFrameCommitted = false; } @@ -292,21 +308,9 @@ export class TimelineFrameModel { const timestamp = event.startTime; if (event.name === RecordType.BeginFrame) { - this.handleBeginFrame(timestamp); + this.handleBeginFrame(timestamp, event.args['frameSeqId']); } else if (event.name === RecordType.DrawFrame) { - if (event.phase === 'I') { - // Legacy behavior: If DrawFrame is an instant event, then it is not - // supposed to contain frame presentation info; use the event time of - // DrawFrame in this case. - // TODO(mjzhang): Remove this legacy support when the migration to - // using presentation time as frame boundary is stablized. - this.handleDrawFrame(timestamp); - } else if (event.args['presentationTimestamp']) { - // Current behavior: Use the presentation timestamp. If the non-instant - // DrawFrame event contains no such timestamp, then the presentation did - // not happen and therefore the event will not be processed. - this.handleDrawFrame(event.args['presentationTimestamp'] / 1000); - } + this.handleDrawFrame(timestamp, event.args['frameSeqId']); } else if (event.name === RecordType.ActivateLayerTree) { this.handleActivateLayerTree(); } else if (event.name === RecordType.RequestMainThreadFrame) { @@ -314,7 +318,7 @@ export class TimelineFrameModel { } else if (event.name === RecordType.NeedsBeginFrameChanged) { this.handleNeedFrameChanged(timestamp, event.args['data'] && event.args['data']['needsBeginFrame']); } else if (event.name === RecordType.DroppedFrame) { - this.handleDroppedFrame(timestamp); + this.handleDroppedFrame(timestamp, event.args['frameSeqId']); } } @@ -535,3 +539,78 @@ export class PendingFrame { this.triggerTime = triggerTime; } } + +// The parameters of an impl-side BeginFrame. +class BeginFrameInfo { + seqId: number; + startTime: number; + isDropped: boolean; + constructor(seqId: number, startTime: number, isDropped: boolean) { + this.seqId = seqId; + this.startTime = startTime; + this.isDropped = isDropped; + } +} + +// A queue of BeginFrames pending visualization. +// BeginFrames are added into this queue as they occur; later when their +// corresponding DrawFrames occur (or lack thereof), the BeginFrames are removed +// from the queue and their timestamps are used for visualization. +export class TimelineFrameBeginFrameQueue { + private queueFrames!: number[]; + + // Maps frameSeqId to BeginFrameInfo. + private mapFrames!: { + [x: number]: BeginFrameInfo, + }; + + constructor() { + this.queueFrames = []; + this.mapFrames = {}; + } + + // Add a BeginFrame to the queue, if it does not already exit. + addFrameIfNotExists(seqId: number, startTime: number, isDropped: boolean): void { + if (!(seqId in this.mapFrames)) { + this.mapFrames[seqId] = new BeginFrameInfo(seqId, startTime, isDropped); + this.queueFrames.push(seqId); + } + } + + // Set a BeginFrame in queue as dropped. + setDropped(seqId: number, isDropped: boolean): void { + if (seqId in this.mapFrames) { + this.mapFrames[seqId].isDropped = isDropped; + } + } + + processPendingBeginFramesOnDrawFrame(seqId: number): BeginFrameInfo[] { + const framesToVisualize: BeginFrameInfo[] = []; + + // Do not visualize this frame in the rare case where the current DrawFrame + // does not have a corresponding BeginFrame. + if (seqId in this.mapFrames) { + // Pop all BeginFrames before the current frame, and add only the dropped + // ones in |frames_to_visualize|. + // Non-dropped frames popped here are BeginFrames that are never + // drawn (but not considered dropped either for some reason). + // Those frames do not require an proactive visualization effort and will + // be naturally presented as continuationss of other frames. + while (this.queueFrames[0] !== seqId) { + const currentSeqId = this.queueFrames[0]; + if (this.mapFrames[currentSeqId].isDropped) { + framesToVisualize.push(this.mapFrames[currentSeqId]); + } + + delete this.mapFrames[currentSeqId]; + this.queueFrames.shift(); + } + + // Pop the BeginFrame associated with the current DrawFrame. + framesToVisualize.push(this.mapFrames[seqId]); + delete this.mapFrames[seqId]; + this.queueFrames.shift(); + } + return framesToVisualize; + } +} diff --git a/test/unittests/front_end/models/timeline_model/BUILD.gn b/test/unittests/front_end/models/timeline_model/BUILD.gn index 2372d22e9a..dd1d8e7444 100644 --- a/test/unittests/front_end/models/timeline_model/BUILD.gn +++ b/test/unittests/front_end/models/timeline_model/BUILD.gn @@ -6,7 +6,10 @@ import("../../../../../third_party/typescript/typescript.gni") ts_library("timeline_model") { testonly = true - sources = [ "TimelineJSProfile_test.ts" ] + sources = [ + "TimelineFrameBeginFrameQueue_test.ts", + "TimelineJSProfile_test.ts", + ] deps = [ "../../../../../front_end/models/timeline_model:bundle" ] } diff --git a/test/unittests/front_end/models/timeline_model/TimelineFrameBeginFrameQueue_test.ts b/test/unittests/front_end/models/timeline_model/TimelineFrameBeginFrameQueue_test.ts new file mode 100644 index 0000000000..d64cc6a61a --- /dev/null +++ b/test/unittests/front_end/models/timeline_model/TimelineFrameBeginFrameQueue_test.ts @@ -0,0 +1,152 @@ +// Copyright 2021 The Chromium Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +const {assert} = chai; + +import * as TimelineModel from '../../../../../front_end/models/timeline_model/timeline_model.js'; + +describe('TimelineFrameBeginFrameQueue', () => { + it('visualizes zero frames when no BeginFrames are added', () => { + const beginFrameQueue = new TimelineModel.TimelineFrameModel.TimelineFrameBeginFrameQueue(); + const framesToVisualize = beginFrameQueue.processPendingBeginFramesOnDrawFrame(100); + + assert.isEmpty(framesToVisualize); + }); + + it('visualizes zero frames when no BeginFrame in queue matches DrawFrame', () => { + const beginFrameQueue = new TimelineModel.TimelineFrameModel.TimelineFrameBeginFrameQueue(); + beginFrameQueue.addFrameIfNotExists(100, 1000000, false); + beginFrameQueue.addFrameIfNotExists(101, 1000016, false); + beginFrameQueue.addFrameIfNotExists(102, 1000032, false); + const framesToVisualize = beginFrameQueue.processPendingBeginFramesOnDrawFrame(103); + + assert.isEmpty(framesToVisualize); + }); + + it('ignores BeginFrames without corresponding DrawFrames', () => { + const beginFrameQueue = new TimelineModel.TimelineFrameModel.TimelineFrameBeginFrameQueue(); + beginFrameQueue.addFrameIfNotExists(100, 1000000, false); + beginFrameQueue.addFrameIfNotExists(101, 1000016, false); + beginFrameQueue.addFrameIfNotExists(102, 1000032, false); + beginFrameQueue.addFrameIfNotExists(103, 1000048, false); + + const framesToVisualize = beginFrameQueue.processPendingBeginFramesOnDrawFrame(102); + + // Visualized frame: 102 (non-dropped). + // The other frames that are neither drawn nor dropped (100, 101) are + // excluded from visualization. + assert.lengthOf(framesToVisualize, 1); + assert.isFalse(framesToVisualize[0].isDropped); + assert.strictEqual(framesToVisualize[0].seqId, 102); + assert.strictEqual(framesToVisualize[0].startTime, 1000032); + }); + + it('visualizes dropped BeginFrames before a presented frame', () => { + const beginFrameQueue = new TimelineModel.TimelineFrameModel.TimelineFrameBeginFrameQueue(); + beginFrameQueue.addFrameIfNotExists(100, 1000000, false); + beginFrameQueue.addFrameIfNotExists(101, 1000016, true); + beginFrameQueue.addFrameIfNotExists(102, 1000032, false); + beginFrameQueue.addFrameIfNotExists(103, 1000048, true); + beginFrameQueue.addFrameIfNotExists(104, 1000064, false); + beginFrameQueue.addFrameIfNotExists(105, 1000080, false); + beginFrameQueue.addFrameIfNotExists(106, 1000096, false); + + const framesToVisualize = beginFrameQueue.processPendingBeginFramesOnDrawFrame(105); + + // Visualized frames: 101 (dropped), 103 (dropped) and 105 (non-dropped). + // The other frames that are neither drawn nor dropped (100, 102 and 104) + // are excluded from visualization. + assert.lengthOf(framesToVisualize, 3); + + assert.isTrue(framesToVisualize[0].isDropped); + assert.strictEqual(framesToVisualize[0].seqId, 101); + assert.strictEqual(framesToVisualize[0].startTime, 1000016); + + assert.isTrue(framesToVisualize[1].isDropped); + assert.strictEqual(framesToVisualize[1].seqId, 103); + assert.strictEqual(framesToVisualize[1].startTime, 1000048); + + assert.isFalse(framesToVisualize[2].isDropped); + assert.strictEqual(framesToVisualize[2].seqId, 105); + assert.strictEqual(framesToVisualize[2].startTime, 1000080); + }); + + it('changes dropped status of specified frames via setDropped()', () => { + const beginFrameQueue = new TimelineModel.TimelineFrameModel.TimelineFrameBeginFrameQueue(); + beginFrameQueue.addFrameIfNotExists(100, 1000000, false); + beginFrameQueue.setDropped(100, true); + beginFrameQueue.addFrameIfNotExists(101, 1000016, true); + beginFrameQueue.addFrameIfNotExists(102, 1000032, false); + beginFrameQueue.addFrameIfNotExists(103, 1000048, true); + beginFrameQueue.addFrameIfNotExists(104, 1000064, false); + beginFrameQueue.addFrameIfNotExists(105, 1000080, false); + beginFrameQueue.addFrameIfNotExists(106, 1000096, true); + beginFrameQueue.setDropped(101, false); + + const framesToVisualize = beginFrameQueue.processPendingBeginFramesOnDrawFrame(105); + + // Visualized frames: 100 (dropped), 103 (dropped) and 105 (non-dropped). + // The other frames that are neither drawn nor dropped (101, 102 and 104) + // are excluded from visualization. + assert.lengthOf(framesToVisualize, 3); + + assert.isTrue(framesToVisualize[0].isDropped); + assert.strictEqual(framesToVisualize[0].seqId, 100); + assert.strictEqual(framesToVisualize[0].startTime, 1000000); + + assert.isTrue(framesToVisualize[1].isDropped); + assert.strictEqual(framesToVisualize[1].seqId, 103); + assert.strictEqual(framesToVisualize[1].startTime, 1000048); + + assert.isFalse(framesToVisualize[2].isDropped); + assert.strictEqual(framesToVisualize[2].seqId, 105); + assert.strictEqual(framesToVisualize[2].startTime, 1000080); + }); + + it('pops processed frames out of the queue', () => { + const beginFrameQueue = new TimelineModel.TimelineFrameModel.TimelineFrameBeginFrameQueue(); + beginFrameQueue.addFrameIfNotExists(100, 1000000, true); + beginFrameQueue.addFrameIfNotExists(101, 1000016, false); + beginFrameQueue.addFrameIfNotExists(102, 1000032, false); + beginFrameQueue.addFrameIfNotExists(103, 1000048, true); + beginFrameQueue.addFrameIfNotExists(104, 1000064, false); + beginFrameQueue.addFrameIfNotExists(105, 1000080, true); + beginFrameQueue.addFrameIfNotExists(106, 1000096, true); + + // Pop frame 100, 101 (not visualized) and 102 from queue. + let framesToVisualize = beginFrameQueue.processPendingBeginFramesOnDrawFrame(102); + + assert.lengthOf(framesToVisualize, 2); + + assert.isTrue(framesToVisualize[0].isDropped); + assert.strictEqual(framesToVisualize[0].seqId, 100); + assert.strictEqual(framesToVisualize[0].startTime, 1000000); + + assert.isFalse(framesToVisualize[1].isDropped); + assert.strictEqual(framesToVisualize[1].seqId, 102); + assert.strictEqual(framesToVisualize[1].startTime, 1000032); + + // Pop frame 103, 104 (not visualized) and 105 from queue + framesToVisualize = beginFrameQueue.processPendingBeginFramesOnDrawFrame(105); + + assert.lengthOf(framesToVisualize, 2); + + assert.isTrue(framesToVisualize[0].isDropped); + assert.strictEqual(framesToVisualize[0].seqId, 103); + assert.strictEqual(framesToVisualize[0].startTime, 1000048); + + assert.isTrue(framesToVisualize[1].isDropped); + assert.strictEqual(framesToVisualize[1].seqId, 105); + assert.strictEqual(framesToVisualize[1].startTime, 1000080); + + // Pop frame 106 from queue + framesToVisualize = beginFrameQueue.processPendingBeginFramesOnDrawFrame(106); + + assert.lengthOf(framesToVisualize, 1); + + assert.isTrue(framesToVisualize[0].isDropped); + assert.strictEqual(framesToVisualize[0].seqId, 106); + assert.strictEqual(framesToVisualize[0].startTime, 1000096); + }); +});