core(gather-runner): warn when BenchmarkIndex is sufficiently low #11350
Conversation
patrickhulce
force-pushed
the
benchmarkindex_warning
branch
from
0905bbd to
0d046f2
Compare
connorjclark
changed the title
connorjclark
changed the title
|
@paulirish are you still concerned about your point in #11350 (comment)? Would love to land this if you're cool with it :) |
paulirish
left a comment
paulirish
left a comment
There was a problem hiding this comment.
This is the new benchmark test page that's quick http://melodic-class.glitch.me/benchmark-quick.html
wait isn't this the older ultradumb benchmark?
just ran it on my (plugged in) laptop.. first time:
in LH we don't get an average... and the first two times it woulda triggered this warning.
so yes i'm still concerned about this being a little too aggressive.
if our BI function has this much variability, i don't think we can add warnings to the user based on it. :/
paulirish
left a comment
paulirish
left a comment
There was a problem hiding this comment.
wait isn't this the older ultradumb benchmark?
I did just attempt at replacing the benchmark code in here with the new computeBenchmarkIndex ... first coupla runs look like they have a bit less variance... plus i'm not seeing any sub 1200 numbers for my laptop anymore. phew.
@patrickhulce can you repro the wide variance i get with the ultradumb? and that there's less with the new one? if so, how about we start with 1000? and then ~next release raise it to 1200 assuming we haven't pissed off everyone yet
Ah shoot, sorry yeah I messed with it when trying something with WPT in between when I posted here and now sorry! https://melodic-class.glitch.me/benchmark-v2-do-not-edit.html should be permanent with the new reminder name to myself to not touch it 😆
The wide variance (it should really just be bimodal with the value being either in the fast bucket or the roughly half as fast slow bucket) with ultradumb is exactly the problem introduced by Chrome 86 that mandated the new version. I can indeed repro it in every environment. The new BenchmarkIndex definitely had less variability than ultradumb (and the modified ultradumb with lower memory footprint) on all machines I tested due to the lower GC dependence.
Sure, wfm. My expectation is that all devices that should be using a 2x multiplier instead of 4x multiplier will not be flagged by a cutoff of 1000 unless many other applications are running on their machine (not even the slowest desktop that takes almost 4x longer to execute JS had a BenchmarkIndex of 1000). |
Co-authored-by: Paul Irish <[email protected]>
docs/throttling.md
Outdated
| lighthouse --throttling.cpuSlowdownMultiplier=6 https://example.com | ||
| ``` | ||
|
|
||
| ## Slow CPU Warnings in the Lighthouse Report |
There was a problem hiding this comment.
is this what you were hoping for @paulirish ?
There was a problem hiding this comment.
i went to town on your markdown. 2e76830 (#11350) so you can review my edits.
but aside from that (and the small items below), this is gtg!
docs/throttling.md
Outdated
| lighthouse --throttling.cpuSlowdownMultiplier=6 https://example.com | ||
| ``` | ||
|
|
||
| ## Slow CPU Warnings in the Lighthouse Report |
| */ | ||
| warningSlowHostCpu: 'The device that ran this test appears to have a slower CPU than ' + | ||
| 'Lighthouse expects. This can negatively affect your performance score. Learn more about using ' + | ||
| '[custom throttling settings](https://github.com/GoogleChrome/lighthouse/blob/master/docs/throttling.md#slow-cpu-warnings-in-the-lighthouse-report) ' + |
There was a problem hiding this comment.
Learn more about [calibrating an appropriate CPU slowdown](https://github.com/GoogleChrome/lighthouse/blob/master/docs/throttling.md#cpu-throttling).
Co-authored-by: Paul Irish <[email protected]>
8 participants
Summary
Adds a toplevel warning when
BenchmarkIndexis sufficiently low (at least ~2x differential in CPU time) when run from the CLI and settings are the defaults.This PR also enables markdown links in our toplevel warnings. It seemed particularly relevant to this message to provide a link on how to act on it, but I think it's a good idea to try to make more warnings actionable if the message doesn't already.
Related Issues/PRs
finally closes #9085