benchmark: ignore significance when using --runs 1

[AndreasMadsen]

Checklist

• commit message follows commit guidelines

Affected core subsystem(s)

benchmark

Description of change

Because the standard deviation can't be calculated when there is only
one observation the R scripts raises an error. However it may still be
useful to run them for non-statistical purposes.

This changes the behaviour such when there is only one observation, the
values that depends on the standard deviation becomes Not Applicable (NA).

Fixes: #8288

/cc @mscdex

---

examples:

                                                               improvement significant   p.value
 http/check_is_http_token.js n=50000000 key="Content-Encoding"      2.42 %          NA        NA
 http/check_is_http_token.js n=50000000 key="Content-encoding"      1.79 %             0.8908776

 chunks length     rate confidence.interval
      0      4 12834.16            5850.699
      1      4 11944.62                  NA

[addaleax]

typo: enogth

[addaleax]

Can’t really review R code, so rubber-stamp LGTM from me.

[mscdex]

I can't really review the code either, but one minor nit is the last line of the commit message is a tad too long (>72).

[AndreasMadsen]

Yeah, I'm not amazing at R either :)

[addaleax]

Hmm… we could give pinging the rest of @nodejs/math aka @fhinkel a try? 😄

[AndreasMadsen]

ping

[addaleax]

If the count remains at 1 rubber-stamp approval (mine), I’d say it’s okay to land this.

I know you’re busy these days, so please don’t feel pressured, but if you’re ever up for porting some of the R code to JS — and if you think that’s feasible – that might be kind of nice for attracting more reviewers. :)

[AndreasMadsen]

@addaleax Great, I will land it then.

I know you’re busy these days, so please don’t feel pressured, but if you’re ever up for porting some of the R code to JS — and if you think that’s feasible – that might be kind of nice for attracting more reviewers. :)

I have already written ttest, so it is feasible. The problem is that it's a big dependency and it's actually super hard to write a correct t-test implementation. When I wrote ttest I checked all other js implementations and found at least on error in each, so I have every reason to expect that ttest is buggy too.

[AndreasMadsen]

CI: https://ci.nodejs.org/job/node-test-pull-request/4074/

[addaleax]

The problem is that it's a big dependency

Yeah, that’s a general problem we have in core. Maybe we should just get a package.json for our top level directory or something…

[AndreasMadsen]

Landed in d3834a1