Bug 195547

Summary: Web Inspector: CPU Usage Timeline - Better Thread chart layout values
Product: WebKit Reporter: Joseph Pecoraro <joepeck>
Component: Web InspectorAssignee: Joseph Pecoraro <joepeck>
Status: RESOLVED FIXED    
Severity: Normal CC: hi, inspector-bugzilla-changes, joepeck, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 194455    
Attachments:
Description Flags
[PATCH] Proposed Fix hi: review+

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>