From 769b156e65ae58c1eb28652e456f76e835fd081d Mon Sep 17 00:00:00 2001
From: sdong <siying.d@fb.com>
Date: Tue, 12 Jul 2022 13:30:35 -0700
Subject: [PATCH] Remove customized naming from InternalKeyComparator (#10343)

Summary:
InternalKeyComparator is a thin wrapper around user comparator. Storing a string for name is relatively expensive to this small wrapper for both CPU and memory usage. Try to remove it.

Pull Request resolved: https://github.com/facebook/rocksdb/pull/10343

Test Plan: Run existing tests

Reviewed By: ajkr

Differential Revision: D37772469

fbshipit-source-id: d2d106a8d022193058fd7f6b220108e3d94aca34
---
 db/compaction/compaction_iterator.h |  2 +-
 db/dbformat.cc                      |  5 +----
 db/dbformat.h                       | 10 ++--------
 db/repair_test.cc                   |  4 +---
 table/block_based/block.h           |  3 +--
 table/sst_file_dumper.cc            |  3 +--
 6 files changed, 7 insertions(+), 20 deletions(-)

diff --git a/db/compaction/compaction_iterator.h b/db/compaction/compaction_iterator.h
index acc6f417f..64145420a 100644
--- a/db/compaction/compaction_iterator.h
+++ b/db/compaction/compaction_iterator.h
@@ -32,7 +32,7 @@ class SequenceIterWrapper : public InternalIterator {
  public:
   SequenceIterWrapper(InternalIterator* iter, const Comparator* cmp,
                       bool need_count_entries)
-      : icmp_(cmp, /*named=*/false),
+      : icmp_(cmp),
         inner_iter_(iter),
         need_count_entries_(need_count_entries) {}
   bool Valid() const override { return inner_iter_->Valid(); }
diff --git a/db/dbformat.cc b/db/dbformat.cc
index 6717341d0..3bb2422dc 100644
--- a/db/dbformat.cc
+++ b/db/dbformat.cc
@@ -117,10 +117,7 @@ std::string InternalKey::DebugString(bool hex) const {
 }
 
 const char* InternalKeyComparator::Name() const {
-  if (name_.empty()) {
-    return "rocksdb.anonymous.InternalKeyComparator";
-  }
-  return name_.c_str();
+  return "rocksdb.anonymous.InternalKeyComparator";
 }
 
 int InternalKeyComparator::Compare(const ParsedInternalKey& a,
diff --git a/db/dbformat.h b/db/dbformat.h
index cdd0285b6..58fc434b4 100644
--- a/db/dbformat.h
+++ b/db/dbformat.h
@@ -238,7 +238,6 @@ class InternalKeyComparator
     : public Comparator {
  private:
   UserComparatorWrapper user_comparator_;
-  std::string name_;
 
  public:
   // `InternalKeyComparator`s constructed with the default constructor are not
@@ -250,13 +249,8 @@ class InternalKeyComparator
   //    this constructor to precompute the result of `Name()`. To avoid this
   //    overhead, set `named` to false. In that case, `Name()` will return a
   //    generic name that is non-specific to the underlying comparator.
-  explicit InternalKeyComparator(const Comparator* c, bool named = true)
-      : Comparator(c->timestamp_size()), user_comparator_(c) {
-    if (named) {
-      name_ = "rocksdb.InternalKeyComparator:" +
-              std::string(user_comparator_.Name());
-    }
-  }
+  explicit InternalKeyComparator(const Comparator* c)
+      : Comparator(c->timestamp_size()), user_comparator_(c) {}
   virtual ~InternalKeyComparator() {}
 
   virtual const char* Name() const override;
diff --git a/db/repair_test.cc b/db/repair_test.cc
index 75b4036bc..993d1f73a 100644
--- a/db/repair_test.cc
+++ b/db/repair_test.cc
@@ -385,9 +385,7 @@ TEST_F(RepairTest, RepairColumnFamilyOptions) {
   ASSERT_OK(db_->GetPropertiesOfAllTables(handles_[1], &fname_to_props));
   ASSERT_EQ(fname_to_props.size(), 2U);
   for (const auto& fname_and_props : fname_to_props) {
-    std::string comparator_name (
-      InternalKeyComparator(rev_opts.comparator).Name());
-    comparator_name = comparator_name.substr(comparator_name.find(':') + 1);
+    std::string comparator_name(rev_opts.comparator->Name());
     ASSERT_EQ(comparator_name,
               fname_and_props.second->comparator_name);
   }
diff --git a/table/block_based/block.h b/table/block_based/block.h
index 531bd4674..32eb12a05 100644
--- a/table/block_based/block.h
+++ b/table/block_based/block.h
@@ -396,8 +396,7 @@ class BlockIter : public InternalIteratorBase<TValue> {
     assert(data_ == nullptr);  // Ensure it is called only once
     assert(num_restarts > 0);  // Ensure the param is valid
 
-    icmp_ =
-        std::make_unique<InternalKeyComparator>(raw_ucmp, false /* named */);
+    icmp_ = std::make_unique<InternalKeyComparator>(raw_ucmp);
     data_ = data;
     restarts_ = restarts;
     num_restarts_ = num_restarts;
diff --git a/table/sst_file_dumper.cc b/table/sst_file_dumper.cc
index 2323b8504..eefbaaeee 100644
--- a/table/sst_file_dumper.cc
+++ b/table/sst_file_dumper.cc
@@ -145,8 +145,7 @@ Status SstFileDumper::GetTableReader(const std::string& file_path) {
                                            &user_comparator);
           if (s.ok()) {
             assert(user_comparator);
-            internal_comparator_ =
-                InternalKeyComparator(user_comparator, /*named=*/true);
+            internal_comparator_ = InternalKeyComparator(user_comparator);
           }
         }
       }