Bug 195547 - Web Inspector: CPU Usage Timeline - Better Thread chart layout values
Summary: Web Inspector: CPU Usage Timeline - Better Thread chart layout values
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Inspector (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: Joseph Pecoraro
URL:
Keywords: InRadar
Depends on:
Blocks: 194455
  Show dependency treegraph
 
Reported: 2019-03-11 01:06 PDT by Joseph Pecoraro
Modified: 2019-03-11 14:53 PDT (History)
4 users (show)

See Also:


Attachments
[PATCH] Proposed Fix (9.62 KB, patch)
2019-03-11 01:09 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-11 01:06:36 PDT
CPU Usage Timeline - Better Thread chart layout values

Rather then layout the thread charts at the maximum usage of all threads, choose a better maximum value to share for the thread views. Try for a multiple for 25 up to 100, otherwise the maximum of a group.
Comment 1 Joseph Pecoraro 2019-03-11 01:09:47 PDT
Created attachment 364240 [details]
[PATCH] Proposed Fix

Won't apply but still reviewable.
Comment 2 Devin Rousso 2019-03-11 01:28:39 PDT
Comment on attachment 364240 [details]
[PATCH] Proposed Fix

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

rs=me

> Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js:463
> +        let workerMax = 0;

Any reason to not use `-Infinity` like the rest?

> Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js:601
> +            if (value > 75)
> +                return 100;
> +            if (value > 50)
> +                return 75;
> +            if (value > 25)
> +                return 50;
> +            return 25;

NIT: this could be rewritten/simplified as `(Math.floor(value / 25) + 1) * 25`.
Comment 3 Joseph Pecoraro 2019-03-11 14:18:55 PDT
Comment on attachment 364240 [details]
[PATCH] Proposed Fix

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

>> Source/WebInspectorUI/UserInterface/Views/CPUTimelineView.js:601
>> +            return 25;
> 
> NIT: this could be rewritten/simplified as `(Math.floor(value / 25) + 1) * 25`.

Yeah, I considered doing this when I first wrote it but I left it as is. This cascade is short and simple and makes it easy to play with new values and tweak.

But I'll switch to this. The arithmetic approach is nice if we want to switch to multiples of 10 (something I've considered as well).
Comment 4 Joseph Pecoraro 2019-03-11 14:30:57 PDT
https://trac.webkit.org/r242740
Comment 5 Radar WebKit Bug Importer 2019-03-11 14:53:15 PDT
<rdar://problem/48784034>