-
Notifications
You must be signed in to change notification settings - Fork 77
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
With privacy.resistFingerprinting set to true in Firefox, Speedometer shows the result Infinity #399
Comments
The simplest fix here IMO would be to detect an infinity score and instead show an error message |
I could imagine a warmup step where it looks for anomalies and shows a warning, like MotionMark does with refresh rate, but this case is likely rare in practice so a guardrail on the score display may be sufficient. |
But I don't understand why one single substep being at 0 would yield an Infinity result. I'm not great at statistics, but I smell something fishy here. |
(update: I got it wrong, I rewrote the comment)
The problem isn't a single substep, but all substeps for 1 test being 0. I've seen it happening for TodoMVC-Preact-Complex-DOM for example, but svelte-complex and lit-complex could happen too. The geomean is a product, so a suite at 0 poisons the geomean for this run. The wikipedia page for geomean says:
Therefore I wonder if the use of geomean is appropriate. A quick fix might be to replace 0 by 1 when adding values in Speedometer/resources/benchmark-runner.mjs Lines 530 to 531 in bd9ab77
(possibly also in Speedometer/resources/metric.mjs Lines 65 to 69 in bd9ab77
What do you think @camillobruni ? |
Note: I also noticed errors with NaN values in the SVG rendering of the Details view, I believe this comes from the same issue. |
I find our score computation very difficult to follow, where we do the same thing in different ways in different files: benchmark-runner.mjs vs metric.mjs vs Statistics.mjs, with no clear comment about the formula we use to calculate the score. It's very unclear to understand where the geomean that we use as the basis for the score is computed from. |
You are absolutely right :) I kind of kept some backwards compatibility with v2.1 but ultimately we should all migrate to metrics.mjs (the Statistics.mjs is from the original v.21 version as to no alter any existing scoring). So Geomean over all the different tests makes sense to not favour the longer running ones disproportionally (we could spend some effort to make each test run the same length in the future?). Given that this is something that should never happen, we could also just fail and not display anything if the sum of all subtests equal to 0. |
Yeah indeed, 0 is an absurd number in this case. Is it practical to ignore the test instead of failing generally ? |
The point of using geometric mean across suites is to give each suite the same weight. If we used arithmetic mean, we would give more weights to the long running / slow suites. |
This partially fixes WebKit#399 by not display non-finite scores which are caused by 0-measured suite results. - For non-positive or non-finite scores, the following summary page is displayed. Note that the detailed page is still accessible since you might find some useful information there. - Additionally, a console.error is generated for each zero-sum suite
This was reported by some people on social networks. We found that setting privacy.resistFingerprinting to true was triggering this. But this might happen in some other cases too.
When doing it, some subtests take 0ms on every run, this may mess up the results.
Any chance we can fix this?
The text was updated successfully, but these errors were encountered: