Bug 196421 - Web Inspector: CPU Usage Timeline - Adjust Energy Impact Threshholds
Summary: Web Inspector: CPU Usage Timeline - Adjust Energy Impact Threshholds
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
Keywords: InRadar
Depends on:
Reported: 2019-03-29 20:18 PDT by Joseph Pecoraro
Modified: 2019-04-01 11:55 PDT (History)
4 users (show)

See Also:

[PATCH] Proposed Fix (6.03 KB, patch)
2019-03-29 20:21 PDT, Joseph Pecoraro
drousso: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Joseph Pecoraro 2019-03-29 20:18:20 PDT
CPU Usage Timeline - Adjust Energy Impact Thresholds

  • Low - Keep Below 3% to continue to encourage idle pages stay below 3%
    NOTE: Might want to reduce to 2.5 or 2 after fixing bug 196419

  • High - Make Above 30% instead of 50% to encourage long running interactivity to stay below 30%
    NOTE: Obviously depends on interaction, but sustained (1-2min) at 30%+ will certainly impact battery

  • Very High - Make above 100% instead of 150%
    NOTE: CPU Usage spikes around page load and is quite often still under 100% despite many threads. Drop this a bit as we dropped High down.
Comment 1 Joseph Pecoraro 2019-03-29 20:21:03 PDT Comment hidden (obsolete)
Comment 2 Joseph Pecoraro 2019-03-29 20:21:53 PDT
Created attachment 366348 [details]
[PATCH] Proposed Fix
Comment 3 Radar WebKit Bug Importer 2019-03-29 20:22:05 PDT Comment hidden (obsolete)
Comment 4 Joseph Pecoraro 2019-03-29 20:24:24 PDT
Comment 5 Devin Rousso 2019-04-01 10:30:37 PDT
Comment on attachment 366348 [details]
[PATCH] Proposed Fix

View in context: https://bugs.webkit.org/attachment.cgi?id=366348&action=review


> Source/WebInspectorUI/ChangeLog:19
> +        - Reduce the size of the Medium section, and increase the High section

Missing "." at the end of this line.  I'd also remove the "-" and indent, as this "point" isn't related to a Low/High/Very High, and is instead a more general comment (e.g. it's not part of a "list").

> Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js:1106
>              // Low. (<=3% CPU, mapped to 0-10)

Given that these comments will have to be updated any time we change the thresholds, I think it might be "easier" to just remove them altogether.  Each "section" is already semi-obvious by the `UIString`s in each block.

> Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js:1109
> +            this._energyChart.value = mapWithBias(average, 0, CPUTimelineView.lowEnergyThreshold, 0, CPUTimelineView.lowEnergyGraphBoundary, 0.85);

Do you also want to make the biases into `static get`?
Comment 6 Joseph Pecoraro 2019-04-01 11:55:15 PDT