⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 243166 in webkit


Ignore:
Timestamp:
Mar 19, 2019, 1:13:03 PM (7 years ago)
Author:
Devin Rousso
Message:

Web Inspector: CPU Usage Timeline - the right edge of each column should align with a CPU measurement
​https://bugs.webkit.org/show_bug.cgi?id=195789
<rdar://problem/48915271>

Reviewed by Joseph Pecoraro.

Right now, each column is rendered such that the middle of the column is aligned with the
time of the CPU measurement. This could potentially be misleading, as the width/position of
the bar implies that there was a period of time after the actual time of the CPU measurement
that should be "attributed" to that same CPU measurement.

1 2 3

_
[ * ]
[ * ]_
[ * ][ * ]

_[ * ][ * ]
[ * ][ * ][ * ]
[*][*][*]

A B C D E F

In this example, one might "attribute" any work done at time B to record 1, when in reality,
it should be "attributed" to record 2, since the CPU measurement had already been taken by
the time B was captured, meaning that the work for B hadn't yet been done and could
therefore not have affected the CPU measurement for record 1.

We should be rendering the columns such that the CPU measurement aligns with the trailing
edge of the column, so that all of the work that could be "attributed" to a given CPU
measurement comes before it.

1 2 3

_ _
[ *] [
[ *]_[
[ *][ *][

[ *][ *][

*][ *][ *][

*][*][*][

A B C D E F

NOTE: this "rendering" isn't exactly accurate, as the * should overlap the ].

Legend:

  • [ ] represents a column for a CPU measurement
  • * represents the time when the measurement actually takes place
  • UserInterface/Views/CPUTimelineOverviewGraph.js:

(WI.CPUTimelineOverviewGraph.prototype.layout):
(WI.CPUTimelineOverviewGraph.prototype._handleChartClick):

  • UserInterface/Views/TimelineOverview.js:

(WI.TimelineOverview.prototype._recordSelected):

Location:
trunk/Source/WebInspectorUI
Files:
3 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/WebInspectorUI/ChangeLog

    r243161 r243166  
     12019-03-19  Devin Rousso  <drousso@apple.com>
     2
     3        Web Inspector: CPU Usage Timeline - the right edge of each column should align with a CPU measurement
     4        https://bugs.webkit.org/show_bug.cgi?id=195789
     5        <rdar://problem/48915271>
     6
     7        Reviewed by Joseph Pecoraro.
     8
     9        Right now, each column is rendered such that the middle of the column is aligned with the
     10        time of the CPU measurement. This could potentially be misleading, as the width/position of
     11        the bar implies that there was a period of time after the actual time of the CPU measurement
     12        that should be "attributed" to that same CPU measurement.
     13
     14           1      2      3
     15               _______
     16               [  *  ]
     17               [  *  ]_______
     18               [  *  ][  *  ]
     19        _______[  *  ][  *  ]
     20        [  *  ][  *  ][  *  ]
     21        [__*__][__*__][__*__]
     22         A   B  C   D  E   F
     23
     24        In this example, one might "attribute" any work done at time B to record 1, when in reality,
     25        it should be "attributed" to record 2, since the CPU measurement had already been taken by
     26        the time B was captured, meaning that the work for B hadn't yet been done and could
     27        therefore not have affected the CPU measurement for record 1.
     28
     29        We should be rendering the columns such that the CPU measurement aligns with the trailing
     30        edge of the column, so that all of the work that could be "attributed" to a given CPU
     31        measurement comes before it.
     32
     33          1      2      3
     34            _______       ___
     35            [    *]       [
     36            [    *]_______[
     37            [    *][    *][
     38        ____[    *][    *][
     39          *][    *][    *][
     40        __*][____*][____*][__
     41         A   B  C   D  E   F
     42
     43                NOTE: this "rendering" isn't exactly accurate, as the `*` should overlap the `]`.
     44
     45        Legend:
     46         - `[     ]` represents a column for a CPU measurement
     47         - `*` represents the time when the measurement actually takes place
     48
     49        * UserInterface/Views/CPUTimelineOverviewGraph.js:
     50        (WI.CPUTimelineOverviewGraph.prototype.layout):
     51        (WI.CPUTimelineOverviewGraph.prototype._handleChartClick):
     52        * UserInterface/Views/TimelineOverview.js:
     53        (WI.TimelineOverview.prototype._recordSelected):
     54
    1552019-03-19  Devin Rousso  <drousso@apple.com>
    256

  • trunk/Source/WebInspectorUI/UserInterface/Views/CPUTimelineOverviewGraph.js

    r243024 r243166  
    117117
    118118        const includeRecordBeforeStart = true;
    119         let visibleRecords = this._cpuTimeline.recordsInTimeRange(graphStartTime, visibleEndTime + (CPUTimelineOverviewGraph.samplingRatePerSecond / 2), includeRecordBeforeStart);
     119        let visibleRecords = this._cpuTimeline.recordsInTimeRange(graphStartTime, visibleEndTime, includeRecordBeforeStart);
    120120        if (!visibleRecords.length)
    121121            return;
    … …  
    125125        }
    126126
    127         let intervalWidth = (CPUTimelineOverviewGraph.samplingRatePerSecond / secondsPerPixel);
     127        let intervalWidth = CPUTimelineOverviewGraph.samplingRatePerSecond / secondsPerPixel;
    128128        const minimumDisplayHeight = 4;
    129129
    130         // Bars for each record.
    131130        for (let record of visibleRecords) {
    132131            let additionalClass = record === this.selectedRecord ? "selected" : undefined;
    133132            let w = intervalWidth;
    134             let x = xScale(record.startTime - (CPUTimelineOverviewGraph.samplingRatePerSecond / 2));           
     133            let x = xScale(record.startTime - CPUTimelineOverviewGraph.samplingRatePerSecond);
    135134            let h1 = Math.max(minimumDisplayHeight, yScale(record.mainThreadUsage));
    136135            let h2 = Math.max(minimumDisplayHeight, yScale(record.mainThreadUsage + record.workerThreadUsage));
    … …  
    201200
    202201        let clickTime = graphStartTime + graphClickTime;
    203         let record = this._cpuTimeline.closestRecordTo(clickTime);
     202        let record = this._cpuTimeline.closestRecordTo(clickTime + (CPUTimelineOverviewGraph.samplingRatePerSecond / 2));
    204203        if (!record)
    205204            return;

  • trunk/Source/WebInspectorUI/UserInterface/Views/TimelineOverview.js

    r243024 r243166  
    789789            if (firstRecord instanceof WI.CPUTimelineRecord) {
    790790                let selectionPadding = WI.CPUTimelineOverviewGraph.samplingRatePerSecond * 2.25;
    791                 this.selectionStartTime = startTime - selectionPadding;
     791                this.selectionStartTime = startTime - selectionPadding - (WI.CPUTimelineOverviewGraph.samplingRatePerSecond / 2);
    792792                this.selectionDuration = endTime - startTime + (selectionPadding * 2);
    793793            } else if (startTime < this.selectionStartTime || endTime > this.selectionStartTime + this.selectionDuration) {
Note: See TracChangeset for help on using the changeset viewer.