[dopiera]

When picking a random size for a buffer from the range 128K and 64M a
uniform distribution is prohibitively wasteful. It's unlikely that one
will need to decide whether 48M or 48.2M is better, but quite likely
need to decide between 128K and 256K. This PR introduces a distribution
which will produce twice as many samples between 0 and 1 than between 1
and 2 and so on.

---

This change is 

[codecov]

Codecov Report

Merging #2942 into master will increase coverage by 0.06%.
The diff coverage is 82.75%.

@@            Coverage Diff             @@
##           master    #2942      +/-   ##
==========================================
+ Coverage   90.56%   90.63%   +0.06%     
==========================================
  Files         298      298              
  Lines       20148    20199      +51     
==========================================
+ Hits        18248    18308      +60     
+ Misses       1900     1891       -9

Impacted Files	Coverage Δ
google/cloud/storage/benchmarks/benchmark_utils.cc	98.66% <100%> (+0.05%)	⬆️
google/cloud/storage/benchmarks/benchmark_utils.h	100% <100%> (ø)	⬆️
.../benchmarks/storage_throughput_vs_cpu_benchmark.cc	71.42% <73.68%> (-0.7%)	⬇️
google/cloud/storage/internal/complex_option.h	66.66% <0%> (-16.67%)	⬇️
...le/cloud/bigtable/internal/async_retry_unary_rpc.h	64.15% <0%> (-1.24%)	⬇️
google/cloud/storage/internal/logging_client.cc	97.76% <0%> (-0.02%)	⬇️
google/cloud/bigtable/row_range.h	100% <0%> (ø)	⬆️
google/cloud/bigtable/mutations.h	100% <0%> (ø)	⬆️
...gle/cloud/storage/internal/access_control_common.h	100% <0%> (ø)	⬆️
google/cloud/bigtable/metadata_update_policy.h	100% <0%> (ø)	⬆️
... and 8 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b319bea...0d901f9. Read the comment docs.

[coryan]

Generally, I am terrified of writing numeric code, and doubly terrified of writing code that combines integer and floating precision arithmetic. Is there any way we can implement this by using some code from the standard library? Or can we approximate this at least by using code from the standard library?

Reviewed 6 of 6 files at r1.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @dopiera)

---

google/cloud/storage/benchmarks/benchmark_utils.h, line 21 at r1 (raw file):

#include "google/cloud/optional.h"
#include <chrono>
#include <cmath>

That seems like it is needed in the .cc file, but not in the .h file?

---

google/cloud/storage/benchmarks/benchmark_utils.h, line 129 at r1 (raw file):

 * distribution.
 *
 * @par Rationale

I get the rationale, but I am not sure this is exactly what we want either. I think we should consider:

• https://en.wikipedia.org/wiki/Stratified_sampling divide the population in exponentially larger groups (for example: [0, 256KiB), [256KiB, 512KiB), [512KiB, 1MiB), ...) until we have enough groups and take N samples from each group
• Use a geometric distribution, which sort of approximates what you do here https://en.wikipedia.org/wiki/Geometric_distribution, https://en.wikipedia.org/wiki/Geometric_distribution
• Populate an array with the desired weights and use a discrete distribution: https://en.cppreference.com/w/cpp/numeric/random/discrete_distribution

---

google/cloud/storage/benchmarks/benchmark_utils.h, line 140 at r1 (raw file):

 * @par The formulas
 * PDF:
 * 1 / (x+1) / (ln(max+1) - ln(min+1))

For what is worth, I think Doxygen supports LaTeX formulas:

http://www.doxygen.nl/manual/formulas.html

---

google/cloud/storage/benchmarks/benchmark_utils.h, line 155 at r1 (raw file):

  template <class Generator>
  result_type operator()(Generator& g) {
    return InvCDF((static_cast<double>(g()) - g.min()) / (g.max() - g.min()));

I guess you could use https://en.cppreference.com/w/cpp/numeric/random/uniform_real_distribution to convert the output of the generator into the desired range?

---

google/cloud/storage/benchmarks/benchmark_utils.cc, line 292 at r1 (raw file):

double SmallValuesBiasedDistribution::PDF(result_type x) const {
  return 1. / (x + 1) / (l_max_ - l_min_);

I will not comment on how numerically stable are these formulas, I will just leave this here:

https://hal.archives-ouvertes.fr/hal-00576641v1/document

and slooooowly step back from it...

---

google/cloud/storage/benchmarks/benchmark_utils.cc, line 296 at r1 (raw file):

double SmallValuesBiasedDistribution::CDF(result_type x) const {
  return (std::log(x + 1) - l_min_) / (l_max_ - l_min_);

I guess l_max_ == l_min_ is UB?

---

google/cloud/storage/benchmarks/benchmark_utils_test.cc, line 63 at r1 (raw file):

  SmallValuesBiasedDistribution::result_type M = 100000;
  SmallValuesBiasedDistribution d(m, M);
  google::cloud::internal::DefaultPRNG generator =

auto generator = maybe?

---

google/cloud/storage/benchmarks/benchmark_utils_test.cc, line 75 at r1 (raw file):

              sum / num_samples, sum / 100);
}

An interesting test to write, look at the interval [16, 5TiB): does the distribution ever produce values outside that range? What about [16, 1EiB)? Hint: double precision numbers only have 52-bits of mantissa, you need 52 for 5TiB, and 60 digits for 1EiB... So 5TiB-16 == 5TiB.

[coryan]

Per our IM discussion, I am going to close this PR. We can reopen it if we change our minds.