Skip to content
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

Fix per-run combine time (fixes #181) #182

Merged
merged 2 commits into from
Feb 1, 2018
Merged

Fix per-run combine time (fixes #181) #182

merged 2 commits into from
Feb 1, 2018

Conversation

patrickdoc
Copy link
Contributor

Before change:

benchmarking perRun
measurement took 231495 s (60 hours!!!)
analysing with 1000 resamples
bootstrapping with 13 of 22 samples (59%)
time                 3.112 ms   (3.102 ms .. 3.122 ms)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 3.119 ms   (3.111 ms .. 3.122 ms)
std dev              10.00 μs   (5.617 μs .. 16.74 μs)
found 1 outliers among 13 samples (7.7%)
  1 (7.7%) low severe
variance introduced by outliers: 4% (slightly inflated)

After:

benchmarking perRun
measurement took 5.135 s
analysing with 1000 resamples
bootstrapping with 33 of 42 samples (78%)
time                 3.120 ms   (3.116 ms .. 3.124 ms)
                     1.000 R²   (1.000 R² .. 1.000 R²)
mean                 3.117 ms   (3.111 ms .. 3.120 ms)
std dev              11.95 μs   (6.974 μs .. 20.56 μs)
found 3 outliers among 33 samples (9.1%)
  3 (9.1%) low severe
variance introduced by outliers: 2% (slightly inflated)

The reported time is now much more accurate, and we get double the number samples.

Copy link
Member

@RyanGlScott RyanGlScott left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

As always, make sure to add an entry to the changelog describing what's different.

@@ -197,7 +197,7 @@ measure bm iters = runBenchmarkable bm iters addResults $ \ !n act -> do
return (m, endTime)
where
addResults :: (Measured, Double) -> (Measured, Double) -> (Measured, Double)
addResults (!m1, !d1) (!m2, !d2) = (m3, d1 + d2)
addResults (!m1, _) (!m2, !d2) = (m3, d2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might also be wise to add a comment describing why we add the Measureds together, but don't add the times together.

@patrickdoc
Copy link
Contributor Author

Done and done. I promise I will remember the changelog in the future 😣

@RyanGlScott RyanGlScott merged commit d957c82 into haskell:master Feb 1, 2018
@patrickdoc patrickdoc deleted the time-comb branch February 2, 2018 18:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants