[PATCH] D16419: Use std::piecewise_constant_distribution instead of ad-hoc binary search.

[Ivan Krasin via llvm-commits]

krasin added a comment.

Please, take another look.


================
Comment at: lib/Fuzzer/FuzzerLoop.cpp:417
@@ -413,15 +416,3 @@
 size_t Fuzzer::ChooseUnitIdxToMutate() {
-    size_t N = Corpus.size();
-    size_t Total = (N + 1) * N / 2;
-    size_t R = USF.GetRand()(Total);
-    size_t IdxBeg = 0, IdxEnd = N;
-    // Binary search.
-    while (IdxEnd - IdxBeg >= 2) {
-      size_t Idx = IdxBeg + (IdxEnd - IdxBeg) / 2;
-      if (R > (Idx + 1) * Idx / 2)
-        IdxBeg = Idx;
-      else
-        IdxEnd = Idx;
-    }
-    assert(IdxBeg < N);
-    return IdxBeg;
+    size_t Idx = size_t(CorpusDistribution(USF.GetRand()));
+    assert(Idx < Corpus.size());
----------------
kcc wrote:
> please use an explicit cast operator. 
> Are you sure it will ever return the corner values (0 and N-1)? 
I've taken a closer look at the generated values. One bug found (not here, though).
This is what I get (histogram for a corpus of size 15)

  16606 0
  33392 1
  49472 2
  66160 3
  82898 4
  99791 5
 116215 6
 132234 7
 149854 8
 166098 9
 183056 10
 199628 11
 216078 12
 232118 13
 248708 14

================
Comment at: lib/Fuzzer/FuzzerLoop.cpp:507
@@ +506,3 @@
+void Fuzzer::UpdateCorpusDistribution() {
+  std::vector<double> Intervals { 0 };
+  std::vector<double> Weights;
----------------
kcc wrote:
> why do you need two vectors here, given that their values are the same? 
> 
> please avoid sp many push backs, just create a properly sized vector. 
> 
> the following loop looks like std::iota, please replace with such. 
> why do you need two vectors here, given that their values are the same?

I need two vectors because one is for intervals and another one for weights.
They are coincidentally similar. That will not be the case, when dynamic weights are introduced.
See http://en.cppreference.com/w/cpp/numeric/random/piecewise_constant_distribution

> please avoid sp many push backs, just create a properly sized vector.
Done. Thank you.

> the following loop looks like std::iota, please replace with such.
Sorry, I don't understand the request. Can you please rephrase the request or give an example?



http://reviews.llvm.org/D16419