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: facebook/rocksdb#10343

Test Plan: Run existing tests

Reviewed By: ajkr

Differential Revision: D37772469

fbshipit-source-id: d2d106a8d022193058fd7f6b220108e3d94aca34

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(); }

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,

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;

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);
   }

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;

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);
           }
         }
       }