chore: minor fixes around huffman encoding (#6037)

Instead of limiting each count in the histogram, we limit and normalize by the histogram sum.

Signed-off-by: Roman Gershman <[email protected]>

Author: romange

src/core/dfly_core_test.cc

@@ -405,11 +405,14 @@ TEST_F(HuffCoderTest, HugeHistogram) {
   // The bug is in the following code in huf_compress.c:
   // huffNode0[0].count = (U32)(1U<<31);  /* fake entry, strong barrier */
   // where it uses the count as a sentinel assuming that no other counts can be larger than 2^31.
-  // this may not be true for histograms with huge counts, so we need to make sure that
-  // all counts are smaller than 2^31 / 256.
+  // this may not be true for histograms with huge counts, so we need to make sure that sum of all
+  // counts is smaller than 2^31.
+  uint64_t sum = 0;
   for (unsigned i = 0; i < hist.size(); ++i) {
-    hist[i] >>= 2;  // Without this the algorithm causes a data race and crash.
+    sum += hist[i];
+    hist[i] /= 4;  // Without this the algorithm causes a data race and crash.
   }
+  LOG(INFO) << "Total sum: " << sum << " reduced sum: " << sum / 4;
   ASSERT_TRUE(encoder_.Build(hist.data(), hist.size() - 1, &error_msg_)) << error_msg_;
 
   string bindata = encoder_.Export();

src/server/debugcmd.cc

@@ -283,8 +283,8 @@ unsigned HufHist::MaxFreqCount() const {
   return max_freq;
 }
 
-unsigned kMaxFreqPerShard = 1U << 19;
-unsigned kMaxFreqTotal = 1U << 23;
+unsigned kMaxFreqPerShard = 1U << 20;
+unsigned kMaxFreqTotal = static_cast<unsigned>((1U << 31) * 0.9);
 
 void DoComputeHist(CompactObjType type, EngineShard* shard, ConnectionContext* cntx,
                    HufHist* dest) {
@@ -1403,23 +1403,20 @@ void DebugCmd::Values(CmdArgList args, facade::SinkReplyBuilder* builder) {
 }
 
 static size_t PostProcessHist(HufHist* dest) {
-  size_t raw_size = 0;
+  size_t total_freq = 0;
   auto& hist = dest->hist;
   unsigned max_freq = 0;
 
   for (unsigned i = 0; i <= HufHist::kMaxSymbol; i++) {
     // raw_size may count less characters than the actual size because
     // we may cut the counting early.
-    raw_size += hist[i];
-    if (hist[i] > max_freq) {
-      max_freq = hist[i];
-    }
+    total_freq += hist[i];
     if (hist[i] == 0) {
       hist[i] = 1;  // Avoid zero frequency symbols.
     }
   }
 
-  if (max_freq > kMaxFreqTotal) {
+  if (total_freq > kMaxFreqTotal) {
     // huffman encoder has a bug with frequencies too high, so we scale down everything
     // to avoid overflow.
     double scale = static_cast<double>(max_freq) / kMaxFreqTotal;
@@ -1430,7 +1427,7 @@ static size_t PostProcessHist(HufHist* dest) {
       }
     }
   }
-  return raw_size;
+  return total_freq;
 }
 
 void DebugCmd::Compression(CmdArgList args, facade::SinkReplyBuilder* builder) {