[PATCH] D73776: Entropic: Boosting LibFuzzer Performance

[Kostya Serebryany via Phabricator via llvm-commits]

kcc added a comment.

Sorry for the delay. Mostly naming/style nits left.



================
Comment at: compiler-rt/lib/fuzzer/FuzzerCorpus.h:42
+  // Power schedule.
+  bool NeedsUpdate = false;
+  double Energy = 0.0;
----------------
NeedsEnergyUpdate?


================
Comment at: compiler-rt/lib/fuzzer/FuzzerCorpus.h:56
+
+    if (lower != FeatureFreqs.end() && lower->first == Idx) {
+      FeatureFreqs.erase(lower);
----------------
Lower 
(use upper case)


================
Comment at: compiler-rt/lib/fuzzer/FuzzerCorpus.h:106
+    // Binary search over local feature frequencies sorted by index.
+    auto lower = std::lower_bound(FeatureFreqs.begin(), FeatureFreqs.end(),
+                                  std::pair<uint32_t, uint16_t>(Idx, 0));
----------------
Lower


================
Comment at: compiler-rt/lib/fuzzer/FuzzerCorpus.h:127
+  // Set in constructor
+  bool Entropic;
+  size_t ConsideredRare;
----------------
I'd like to put these into a 
  struct EntropicOptions {
     bool Enabled;
     size_t EntropicFeatureFrequencyThreshold;
    ...
  }

and pass such a struct (by value) to InputCorpus CTOR.
Of course, please match the names with those in Options


================
Comment at: compiler-rt/lib/fuzzer/FuzzerOptions.h:47
   bool OnlyASCII = false;
+  bool Entropic = false;
+  size_t ConsideredRare = 0xFF;
----------------
I'd prefer if these names were more descriptive (ok if longer) and have "entropic" in it.
E.g. 
  Entropic
  EntropicFeatureFrequencyThreshold
  EntropicNumberOfRarestFeatures

of course, make the command line parameters match 


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73776/new/

https://reviews.llvm.org/D73776