Bug 197605 - JS2 should print scores for different categories
Summary: JS2 should print scores for different categories
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Saam Barati
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-05-05 23:37 PDT by Saam Barati
Modified: 2019-05-06 13:37 PDT (History)
17 users (show)

See Also:


Attachments
patch (1.83 KB, patch)
2019-05-05 23:42 PDT, Saam Barati
ysuzuki: review+
Details | Formatted Diff | Diff
patch (3.68 KB, patch)
2019-05-06 12:03 PDT, Saam Barati
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Saam Barati 2019-05-05 23:37:28 PDT
...
Comment 1 Saam Barati 2019-05-05 23:42:01 PDT
Created attachment 369101 [details]
patch
Comment 2 Tadeu Zagallo 2019-05-05 23:48:08 PDT
Why is this only for the CLI? Also, the ChangeLog entry is missing.
Comment 3 Yusuke Suzuki 2019-05-05 23:59:17 PDT
Comment on attachment 369101 [details]
patch

r=me. As Tadeu said, adding this feature to non-cli version would be also useful too (yeah, it would involve some UI changes...
And please add ChangeLog for this.
Comment 4 Saam Barati 2019-05-06 11:34:58 PDT
(In reply to Tadeu Zagallo from comment #2)
> Why is this only for the CLI? Also, the ChangeLog entry is missing.

If we add it to the website, we should do it behind some kind of key click when we're done running. Let me look into it.
Comment 5 Saam Barati 2019-05-06 12:03:57 PDT
Created attachment 369153 [details]
patch
Comment 6 Yusuke Suzuki 2019-05-06 12:17:31 PDT
Comment on attachment 369153 [details]
patch

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

r=me

> PerformanceTests/JetStream2/JetStreamDriver.js:53
> +    for (let [category, scores] of categoryScores)
> +        summaryElement.innerHTML += `<p> ${category}: ${uiFriendlyNumber(geomean(scores))}</p>`

I think sorting the categories is better.

> PerformanceTests/JetStream2/JetStreamDriver.js:247
> +            for (let [category, scores] of categoryScores)
> +                console.log(`${category}: ${uiFriendlyNumber(geomean(scores))}`);

Ditto.
Comment 7 Saam Barati 2019-05-06 12:27:35 PDT
Comment on attachment 369153 [details]
patch

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

>> PerformanceTests/JetStream2/JetStreamDriver.js:53
>> +        summaryElement.innerHTML += `<p> ${category}: ${uiFriendlyNumber(geomean(scores))}</p>`
> 
> I think sorting the categories is better.

How would you sort it? It's nice now that benchmarks that share sub categories end up printing next to each other.
Comment 8 Yusuke Suzuki 2019-05-06 13:17:54 PDT
Comment on attachment 369153 [details]
patch

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

>>> PerformanceTests/JetStream2/JetStreamDriver.js:53
>>> +        summaryElement.innerHTML += `<p> ${category}: ${uiFriendlyNumber(geomean(scores))}</p>`
>> 
>> I think sorting the categories is better.
> 
> How would you sort it? It's nice now that benchmarks that share sub categories end up printing next to each other.

OK, talked with Saam. The order is kept by ES6 Map.
Comment 9 WebKit Commit Bot 2019-05-06 13:36:46 PDT
Comment on attachment 369153 [details]
patch

Clearing flags on attachment: 369153

Committed r244973: <https://trac.webkit.org/changeset/244973>
Comment 10 WebKit Commit Bot 2019-05-06 13:36:47 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Radar WebKit Bug Importer 2019-05-06 13:37:21 PDT
<rdar://problem/50511028>