WebKit Bugzilla
Attachment 368617 Details for
Bug 197440
: Web Inspector: Timelines: CPU/memory timeline bars sometimes don't draw correctly and jump around on scrolling
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-197440-20190430151949.patch (text/plain), 18.91 KB, created by
Devin Rousso
on 2019-04-30 15:19:50 PDT
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Devin Rousso
Created:
2019-04-30 15:19:50 PDT
Size:
18.91 KB
patch
obsolete
>diff --git a/Source/WebInspectorUI/ChangeLog b/Source/WebInspectorUI/ChangeLog >index 9dc8d0d299134976d3b6231c068015fb9849519b..8e45f514ec835dc83f5755b90215108b8c8a6821 100644 >--- a/Source/WebInspectorUI/ChangeLog >+++ b/Source/WebInspectorUI/ChangeLog >@@ -1,3 +1,44 @@ >+2019-04-30 Devin Rousso <drousso@apple.com> >+ >+ Web Inspector: Timelines: CPU/memory timeline bars sometimes don't draw correctly and jump around on scrolling >+ https://bugs.webkit.org/show_bug.cgi?id=197440 >+ <rdar://problem/46886315> >+ >+ Reviewed by NOBODY (OOPS!). >+ >+ * UserInterface/Models/Timeline.js: >+ (WI.Timeline.prototype.recordsOverlappingTimeRange): >+ (WI.Timeline.prototype.recordsInTimeRange): Deleted. >+ Merge `recordsInTimeRange` into `recordsOverlappingTimeRange` by automatically including >+ the record before and after the first/last record that are wholly contained by the range. >+ >+ * UserInterface/Models/CPUTimelineRecord.js: >+ (WI.CPUTimelineRecord): >+ (WI.CPUTimelineRecord.get samplingRatePerSecond): Added. >+ * UserInterface/Models/MemoryTimelineRecord.js: >+ (WI.MemoryTimelineRecord): >+ (WI.MemoryTimelineRecord.get samplingRatePerSecond): Added. >+ Adjust the `startTime` of the record by the sampling rate (which is 500ms). >+ >+ * UserInterface/Views/TimelineOverview.js: >+ (WI.TimelineOverview.prototype._recordSelected): >+ * UserInterface/Views/CPUTimelineView.js: >+ (WI.CPUTimelineView.prototype.layout): >+ (WI.CPUTimelineView.prototype._computeStatisticsData): >+ * UserInterface/Views/CPUTimelineOverviewGraph.js: >+ (WI.CPUTimelineOverviewGraph.prototype.layout): >+ (WI.CPUTimelineOverviewGraph.prototype._handleChartClick): >+ (WI.CPUTimelineOverviewGraph.prototype.get samplingRatePerSecond): Deleted. >+ (WI.CPUTimelineOverviewGraph.prototype.layout.yScaleForRecord): Deleted. >+ >+ * UserInterface/Views/MemoryTimelineView.js: >+ (WI.MemoryTimelineView.prototype.layout): >+ * UserInterface/Views/MemoryTimelineOverviewGraph.js: >+ (WI.MemoryTimelineOverviewGraph.prototype.layout): >+ >+ * UserInterface/Views/HeapAllocationsTimelineOverviewGraph.js: >+ (WI.HeapAllocationsTimelineOverviewGraph.prototype.layout): >+ > 2019-04-29 Alex Christensen <achristensen@webkit.org> > > <rdar://problem/50299396> Fix internal High Sierra build >diff --git a/Source/WebInspectorUI/UserInterface/Models/CPUTimelineRecord.js b/Source/WebInspectorUI/UserInterface/Models/CPUTimelineRecord.js >index 2166891e16f828a378c3e98a66920fb9b852e22e..479e679940cc1a0ad5607cfc591b7ba7f3730c6b 100644 >--- a/Source/WebInspectorUI/UserInterface/Models/CPUTimelineRecord.js >+++ b/Source/WebInspectorUI/UserInterface/Models/CPUTimelineRecord.js >@@ -27,7 +27,7 @@ WI.CPUTimelineRecord = class CPUTimelineRecord extends WI.TimelineRecord > { > constructor({timestamp, usage, threads}) > { >- super(WI.TimelineRecord.Type.CPU, timestamp, timestamp); >+ super(WI.TimelineRecord.Type.CPU, timestamp - CPUTimelineRecord.samplingRatePerSecond, timestamp); > > console.assert(typeof timestamp === "number"); > console.assert(typeof usage === "number"); >@@ -68,6 +68,14 @@ WI.CPUTimelineRecord = class CPUTimelineRecord extends WI.TimelineRecord > } > } > >+ // Static >+ >+ static get samplingRatePerSecond() >+ { >+ // 500ms. This matches the ResourceUsageThread sampling frequency in the backend. >+ return 0.5; >+ } >+ > // Import / Export > > static fromJSON(json) >diff --git a/Source/WebInspectorUI/UserInterface/Models/MemoryTimelineRecord.js b/Source/WebInspectorUI/UserInterface/Models/MemoryTimelineRecord.js >index 1e37b9a48aed3624933b45184784f3dfa1e26a05..87427730bccd301b31ffa29634c6dfa28fcbba89 100644 >--- a/Source/WebInspectorUI/UserInterface/Models/MemoryTimelineRecord.js >+++ b/Source/WebInspectorUI/UserInterface/Models/MemoryTimelineRecord.js >@@ -27,7 +27,7 @@ WI.MemoryTimelineRecord = class MemoryTimelineRecord extends WI.TimelineRecord > { > constructor(timestamp, categories) > { >- super(WI.TimelineRecord.Type.Memory, timestamp, timestamp); >+ super(WI.TimelineRecord.Type.Memory, timestamp - MemoryTimelineRecord.samplingRatePerSecond, timestamp); > > console.assert(typeof timestamp === "number"); > console.assert(categories instanceof Array); >@@ -43,6 +43,12 @@ WI.MemoryTimelineRecord = class MemoryTimelineRecord extends WI.TimelineRecord > > // Static > >+ static get samplingRatePerSecond() >+ { >+ // 500ms. This matches the ResourceUsageThread sampling frequency in the backend. >+ return 0.5; >+ } >+ > static memoryCategoriesFromProtocol(categories) > { > let javascriptSize = 0; >diff --git a/Source/WebInspectorUI/UserInterface/Models/Timeline.js b/Source/WebInspectorUI/UserInterface/Models/Timeline.js >index 0be7ec5a77edc70cef940108bdfa07b44caea349..1b3a5fb37cd1add1ae0312fb123b06f2acf0afdd 100644 >--- a/Source/WebInspectorUI/UserInterface/Models/Timeline.js >+++ b/Source/WebInspectorUI/UserInterface/Models/Timeline.js >@@ -116,28 +116,10 @@ WI.Timeline = class Timeline extends WI.Object > let lowerIndex = this._records.lowerBound(startTime, (time, record) => time - record.endTime); > let upperIndex = this._records.upperBound(endTime, (time, record) => time - record.startTime); > >- return this._records.slice(lowerIndex, upperIndex); >- } >- >- recordsInTimeRange(startTime, endTime, includeRecordBeforeStart) >- { >- let lowerIndex = this._records.lowerBound(startTime, (time, record) => time - record.startTime); >- let upperIndex = this._records.upperBound(endTime, (time, record) => time - record.startTime); >- >- // Include the record right before the start time. >- if (includeRecordBeforeStart && lowerIndex > 0) { >- lowerIndex--; >- >- // If the record right before is a child of the same type of record, then use the parent as the before index. >- let recordBefore = this._records[lowerIndex]; >- if (recordBefore.parent && recordBefore.parent.type === recordBefore.type) { >- lowerIndex--; >- while (this._records[lowerIndex] !== recordBefore.parent) >- lowerIndex--; >- } >- } >- >- return this._records.slice(lowerIndex, upperIndex); >+ let constrain = (value) => { >+ return Number.constrain(value, 0, this._records.length); >+ }; >+ return this._records.slice(constrain(lowerIndex - 1), constrain(upperIndex + 1)); > } > > // Private >diff --git a/Source/WebInspectorUI/UserInterface/Views/CPUTimelineOverviewGraph.js b/Source/WebInspectorUI/UserInterface/Views/CPUTimelineOverviewGraph.js >index 087a4a6bf3f600a582809bdf05b0d18b61131c26..72b5d03401e0767f57436d771022d947afaf208f 100644 >--- a/Source/WebInspectorUI/UserInterface/Views/CPUTimelineOverviewGraph.js >+++ b/Source/WebInspectorUI/UserInterface/Views/CPUTimelineOverviewGraph.js >@@ -56,14 +56,6 @@ WI.CPUTimelineOverviewGraph = class CPUTimelineOverviewGraph extends WI.Timeline > this._processRecord(record); > } > >- // Static >- >- static get samplingRatePerSecond() >- { >- // 500ms. This matches the ResourceUsageThread sampling frequency in the backend. >- return 0.5; >- } >- > // Protected > > get height() >@@ -115,22 +107,16 @@ WI.CPUTimelineOverviewGraph = class CPUTimelineOverviewGraph extends WI.Timeline > return (size / maxCapacity) * height; > } > >- const includeRecordBeforeStart = true; >- let visibleRecords = this._cpuTimeline.recordsInTimeRange(graphStartTime, visibleEndTime, includeRecordBeforeStart); >+ let visibleRecords = this._cpuTimeline.recordsOverlappingTimeRange(graphStartTime, visibleEndTime); > if (!visibleRecords.length) > return; > >- function yScaleForRecord(record) { >- return yScale(record.usage); >- } >- >- let intervalWidth = CPUTimelineOverviewGraph.samplingRatePerSecond / secondsPerPixel; > const minimumDisplayHeight = 4; > > for (let record of visibleRecords) { > let additionalClass = record === this.selectedRecord ? "selected" : undefined; >- let w = intervalWidth; >- let x = xScale(record.startTime - CPUTimelineOverviewGraph.samplingRatePerSecond); >+ let w = (record.endTime - record.startTime) / secondsPerPixel; >+ let x = xScale(record.startTime); > let h1 = Math.max(minimumDisplayHeight, yScale(record.mainThreadUsage)); > let h2 = Math.max(minimumDisplayHeight, yScale(record.mainThreadUsage + record.workerThreadUsage)); > let h3 = Math.max(minimumDisplayHeight, yScale(record.usage)); >@@ -199,7 +185,7 @@ WI.CPUTimelineOverviewGraph = class CPUTimelineOverviewGraph extends WI.Timeline > let graphStartTime = this.startTime; > > let clickTime = graphStartTime + graphClickTime; >- let record = this._cpuTimeline.closestRecordTo(clickTime + (CPUTimelineOverviewGraph.samplingRatePerSecond / 2)); >+ let record = this._cpuTimeline.closestRecordTo(clickTime); > if (!record) > return; > >diff --git a/Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js b/Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js >index 1513d4eaca5465a11e439d34258c973cf81f8637..f9fae5d4c4c04b108a12fcbfae89d910f53eb5a2 100644 >--- a/Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js >+++ b/Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js >@@ -441,18 +441,15 @@ WI.CPUTimelineView = class CPUTimelineView extends WI.TimelineView > let graphEndTime = this.endTime; > let visibleEndTime = Math.min(this.endTime, this.currentTime); > let visibleDuration = visibleEndTime - graphStartTime; >- >- let discontinuities = this._recording.discontinuitiesInTimeRange(graphStartTime, visibleEndTime); >- let originalDiscontinuities = discontinuities.slice(); >- >- // Don't include the record before the graph start if the graph start is within a gap. >- let includeRecordBeforeStart = !discontinuities.length || discontinuities[0].startTime > graphStartTime; >- let visibleRecords = this.representedObject.recordsInTimeRange(graphStartTime, visibleEndTime, includeRecordBeforeStart); >+ let visibleRecords = this.representedObject.recordsOverlappingTimeRange(graphStartTime, visibleEndTime); > if (!visibleRecords.length || (visibleRecords.length === 1 && visibleRecords[0].endTime < graphStartTime)) { > this.clear(); > return; > } > >+ let discontinuities = this._recording.discontinuitiesInTimeRange(graphStartTime, visibleEndTime); >+ let originalDiscontinuities = discontinuities.slice(); >+ > this._secondsPerPixelInLayout = secondsPerPixel; > this._visibleRecordsInLayout = visibleRecords; > this._discontinuitiesInLayout = discontinuities.slice(); >@@ -1161,8 +1158,6 @@ WI.CPUTimelineView = class CPUTimelineView extends WI.TimelineView > // with the data available to the frontend and is quite accurate for most > // Main Thread activity. > >- const includeRecordBeforeStart = true; >- > function incrementTypeCount(map, key) { > let entry = map.get(key); > if (entry) >@@ -1183,7 +1178,7 @@ WI.CPUTimelineView = class CPUTimelineView extends WI.TimelineView > let possibleRepeatingTimers = new Set; > > let scriptTimeline = this._recording.timelineForRecordType(WI.TimelineRecord.Type.Script); >- let scriptRecords = scriptTimeline ? scriptTimeline.recordsInTimeRange(startTime, endTime, includeRecordBeforeStart) : []; >+ let scriptRecords = scriptTimeline ? scriptTimeline.recordsOverlappingTimeRange(startTime, endTime) : []; > scriptRecords = scriptRecords.filter((record) => { > // Return true for event types that define script entries/exits. > // Return false for events with no time ranges or if they are contained in other events. >@@ -1249,7 +1244,7 @@ WI.CPUTimelineView = class CPUTimelineView extends WI.TimelineView > }); > > let layoutTimeline = this._recording.timelineForRecordType(WI.TimelineRecord.Type.Layout); >- let layoutRecords = layoutTimeline ? layoutTimeline.recordsInTimeRange(startTime, endTime, includeRecordBeforeStart) : []; >+ let layoutRecords = layoutTimeline ? layoutTimeline.recordsOverlappingTimeRange(startTime, endTime) : []; > layoutRecords = layoutRecords.filter((record) => { > switch (record.eventType) { > case WI.LayoutTimelineRecord.EventType.RecalculateStyles: >@@ -1272,7 +1267,7 @@ WI.CPUTimelineView = class CPUTimelineView extends WI.TimelineView > }); > > let networkTimeline = this._recording.timelineForRecordType(WI.TimelineRecord.Type.Network); >- let networkRecords = networkTimeline ? networkTimeline.recordsInTimeRange(startTime, endTime) : []; >+ let networkRecords = networkTimeline ? networkTimeline.recordsOverlappingTimeRange(startTime, endTime) : []; > let networkRequests = networkRecords.length; > > let millisecondStartTime = Math.round(startTime * 1000); >diff --git a/Source/WebInspectorUI/UserInterface/Views/HeapAllocationsTimelineOverviewGraph.js b/Source/WebInspectorUI/UserInterface/Views/HeapAllocationsTimelineOverviewGraph.js >index 4594ab44393f411b5b031d607146561c20c160a9..acf7d01efa59474cc9e77cd7e3dd02ba8c2f2a32 100644 >--- a/Source/WebInspectorUI/UserInterface/Views/HeapAllocationsTimelineOverviewGraph.js >+++ b/Source/WebInspectorUI/UserInterface/Views/HeapAllocationsTimelineOverviewGraph.js >@@ -60,8 +60,7 @@ WI.HeapAllocationsTimelineOverviewGraph = class HeapAllocationsTimelineOverviewG > this._selectedImageElement = null; > } > >- // This may display records past the current time marker. >- let visibleRecords = this._heapAllocationsTimeline.recordsInTimeRange(this.startTime, this.endTime); >+ let visibleRecords = this._heapAllocationsTimeline.recordsOverlappingTimeRange(this.startTime, this.endTime); > if (!visibleRecords.length) > return; > >diff --git a/Source/WebInspectorUI/UserInterface/Views/MemoryTimelineOverviewGraph.js b/Source/WebInspectorUI/UserInterface/Views/MemoryTimelineOverviewGraph.js >index 496092fff701f06d60e3937ca9067663502c117b..419110ae45b8cae714eb406576373bfad4dc900a 100644 >--- a/Source/WebInspectorUI/UserInterface/Views/MemoryTimelineOverviewGraph.js >+++ b/Source/WebInspectorUI/UserInterface/Views/MemoryTimelineOverviewGraph.js >@@ -136,13 +136,7 @@ WI.MemoryTimelineOverviewGraph = class MemoryTimelineOverviewGraph extends WI.Ti > element.remove(); > } > >- let discontinuities = this.timelineOverview.discontinuitiesInTimeRange(graphStartTime, visibleEndTime); >- >- // Don't include the record before the graph start if the graph start is within a gap. >- let includeRecordBeforeStart = !discontinuities.length || discontinuities[0].startTime > graphStartTime; >- >- // FIXME: <https://webkit.org/b/153759> Web Inspector: Memory Timelines should better extend to future data >- let visibleRecords = this._memoryTimeline.recordsInTimeRange(graphStartTime, visibleEndTime, includeRecordBeforeStart); >+ let visibleRecords = this._memoryTimeline.recordsOverlappingTimeRange(graphStartTime, visibleEndTime); > if (!visibleRecords.length) > return; > >@@ -156,6 +150,8 @@ WI.MemoryTimelineOverviewGraph = class MemoryTimelineOverviewGraph extends WI.Ti > return ys; > } > >+ let discontinuities = this.timelineOverview.discontinuitiesInTimeRange(graphStartTime, visibleEndTime); >+ > // Extend the first record to the start so it doesn't look like we originate at zero size. > if (visibleRecords[0] === this._memoryTimeline.records[0] && (!discontinuities.length || discontinuities[0].startTime > visibleRecords[0].startTime)) > this._chart.addPointSet(0, pointSetForRecord(visibleRecords[0])); >diff --git a/Source/WebInspectorUI/UserInterface/Views/MemoryTimelineView.js b/Source/WebInspectorUI/UserInterface/Views/MemoryTimelineView.js >index 5172770f54666ecdd1922861ca1d2856ffcd71b5..09a4afbfd08c3ccc129af21a682fd4da92ed4c4d 100644 >--- a/Source/WebInspectorUI/UserInterface/Views/MemoryTimelineView.js >+++ b/Source/WebInspectorUI/UserInterface/Views/MemoryTimelineView.js >@@ -194,19 +194,14 @@ WI.MemoryTimelineView = class MemoryTimelineView extends WI.TimelineView > let graphEndTime = this.endTime; > let secondsPerPixel = this._timelineRuler.secondsPerPixel; > let visibleEndTime = Math.min(this.endTime, this.currentTime); >- >- let discontinuities = this._recording.discontinuitiesInTimeRange(graphStartTime, visibleEndTime); >- >- // Don't include the record before the graph start if the graph start is within a gap. >- let includeRecordBeforeStart = !discontinuities.length || discontinuities[0].startTime > graphStartTime; >- >- // FIXME: <https://webkit.org/b/153759> Web Inspector: Memory Timelines should better extend to future data >- let visibleRecords = this.representedObject.recordsInTimeRange(graphStartTime, visibleEndTime, includeRecordBeforeStart); >+ let visibleRecords = this.representedObject.recordsOverlappingTimeRange(graphStartTime, visibleEndTime); > if (!visibleRecords.length || (visibleRecords.length === 1 && visibleRecords[0].endTime < graphStartTime)) { > this.clear(); > return; > } > >+ let discontinuities = this._recording.discontinuitiesInTimeRange(graphStartTime, visibleEndTime); >+ > // Update total usage chart with the last record's data. > let lastRecord = visibleRecords.lastValue; > let values = []; >diff --git a/Source/WebInspectorUI/UserInterface/Views/TimelineOverview.js b/Source/WebInspectorUI/UserInterface/Views/TimelineOverview.js >index 3cec4fb77e7fe2697a0e0a5c60e4e37d886783b7..0adadf62409cf310582bd510de7a1bc3404ee760 100644 >--- a/Source/WebInspectorUI/UserInterface/Views/TimelineOverview.js >+++ b/Source/WebInspectorUI/UserInterface/Views/TimelineOverview.js >@@ -786,8 +786,8 @@ WI.TimelineOverview = class TimelineOverview extends WI.View > let endTime = lastRecord instanceof WI.RenderingFrameTimelineRecord ? lastRecord.frameIndex : lastRecord.endTime; > > if (firstRecord instanceof WI.CPUTimelineRecord) { >- let selectionPadding = WI.CPUTimelineOverviewGraph.samplingRatePerSecond * 2.25; >- this.selectionStartTime = startTime - selectionPadding - (WI.CPUTimelineOverviewGraph.samplingRatePerSecond / 2); >+ let selectionPadding = WI.CPUTimelineRecord.samplingRatePerSecond * 2.25; >+ this.selectionStartTime = startTime - selectionPadding; > this.selectionDuration = endTime - startTime + (selectionPadding * 2); > } else if (startTime < this.selectionStartTime || endTime > this.selectionStartTime + this.selectionDuration) { > let selectionPadding = this.secondsPerPixel * 10;
<b>You cannot view the attachment while viewing its details because your browser does not support IFRAMEs. <a href="attachment.cgi?id=368617">View the attachment on a separate page</a>.</b>
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 197440
:
368617
|
369026
|
369033
|
369053
|
370184
|
370186