Move RegisterOptions into the Configurable API (#8223)

Summary:
As previously coded, a Configurable extension would need access to code not in the public API.  This change moves RegisterOptions into the Configurable class and therefore available to public extensions.

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

Reviewed By: anand1976

Differential Revision: D27960188

Pulled By: mrambacher

fbshipit-source-id: ac88b19397183df633902def5b5701b9b65fbf40

12 files changed, 65 insertions, 86 deletions

diff --git a/include/rocksdb/configurable.h b/include/rocksdb/configurable.h
index 655b3b43d..b56072dbe 100644
--- a/include/rocksdb/configurable.h
+++ b/include/rocksdb/configurable.h
@@ -350,6 +350,35 @@ class Configurable {
   //  Given a name (e.g. rocksdb.my.type.opt), returns the short name (opt)
   virtual std::string GetOptionName(const std::string& long_name) const;
 
+  // Registers the input name with the options and associated map.
+  // When classes register their options in this manner, most of the
+  // functionality (excluding unknown options and validate/prepare) is
+  // implemented by the base class.
+  //
+  // This method should be called in the class constructor to register the
+  // option set for this object.  For example, to register the options
+  // associated with the BlockBasedTableFactory, the constructor calls this
+  // method passing in:
+  // - the name of the options ("BlockBasedTableOptions");
+  // - the options object (the BlockBasedTableOptions object for this object;
+  // - the options type map for the BlockBasedTableOptions.
+  // This registration allows the Configurable class to process the option
+  // values associated with the BlockBasedTableOptions without further code in
+  // the derived class.
+  //
+  // @param name    The name of this set of options (@see GetOptionsPtr)
+  // @param opt_ptr Pointer to the options to associate with this name
+  // @param opt_map Options map that controls how this option is configured.
+  template <typename T>
+  void RegisterOptions(
+      T* opt_ptr,
+      const std::unordered_map<std::string, OptionTypeInfo>* opt_map) {
+    RegisterOptions(T::kName(), opt_ptr, opt_map);
+  }
+  void RegisterOptions(
+      const std::string& name, void* opt_ptr,
+      const std::unordered_map<std::string, OptionTypeInfo>* opt_map);
+
  private:
   // Contains the collection of options (name, opt_ptr, opt_map) associated with
   // this object. This collection is typically set in the constructor of the
diff --git a/options/cf_options.cc b/options/cf_options.cc
index 64a5ae7fb..6332f3035 100644
--- a/options/cf_options.cc
+++ b/options/cf_options.cc
@@ -687,10 +687,9 @@ const std::string OptionsHelper::kCFOptionsName = "ColumnFamilyOptions";
 
 class ConfigurableMutableCFOptions : public Configurable {
  public:
-  ConfigurableMutableCFOptions(const MutableCFOptions& mcf) {
+  explicit ConfigurableMutableCFOptions(const MutableCFOptions& mcf) {
     mutable_ = mcf;
-    ConfigurableHelper::RegisterOptions(*this, &mutable_,
-                                        &cf_mutable_options_type_info);
+    RegisterOptions(&mutable_, &cf_mutable_options_type_info);
   }
 
  protected:
@@ -705,8 +704,7 @@ class ConfigurableCFOptions : public ConfigurableMutableCFOptions {
         immutable_(ImmutableDBOptions(), opts),
         cf_options_(opts),
         opt_map_(map) {
-    ConfigurableHelper::RegisterOptions(*this, &immutable_,
-                                        &cf_immutable_options_type_info);
+    RegisterOptions(&immutable_, &cf_immutable_options_type_info);
   }
 
  protected:
diff --git a/options/configurable.cc b/options/configurable.cc
index aa2b957c4..f425f193c 100644
--- a/options/configurable.cc
+++ b/options/configurable.cc
@@ -17,10 +17,10 @@
 
 namespace ROCKSDB_NAMESPACE {
 
-void ConfigurableHelper::RegisterOptions(
-    Configurable& configurable, const std::string& name, void* opt_ptr,
+void Configurable::RegisterOptions(
+    const std::string& name, void* opt_ptr,
     const std::unordered_map<std::string, OptionTypeInfo>* type_map) {
-  Configurable::RegisteredOptions opts;
+  RegisteredOptions opts;
   opts.name = name;
 #ifndef ROCKSDB_LITE
   opts.type_map = type_map;
@@ -28,7 +28,7 @@ void ConfigurableHelper::RegisterOptions(
   (void)type_map;
 #endif  // ROCKSDB_LITE
   opts.opt_ptr = opt_ptr;
-  configurable.options_.emplace_back(opts);
+  options_.emplace_back(opts);
 }
 
 //*************************************************************************
diff --git a/options/configurable_helper.h b/options/configurable_helper.h
index bc54a099f..b822b0b8e 100644
--- a/options/configurable_helper.h
+++ b/options/configurable_helper.h
@@ -22,35 +22,6 @@ class ConfigurableHelper {
  public:
   constexpr static const char* kIdPropName = "id";
   constexpr static const char* kIdPropSuffix = ".id";
-  // Registers the input name with the options and associated map.
-  // When classes register their options in this manner, most of the
-  // functionality (excluding unknown options and validate/prepare) is
-  // implemented by the base class.
-  //
-  // This method should be called in the class constructor to register the
-  // option set for this object.  For example, to register the options
-  // associated with the BlockBasedTableFactory, the constructor calls this
-  // method passing in:
-  // - the name of the options ("BlockBasedTableOptions");
-  // - the options object (the BlockBasedTableOptions object for this object;
-  // - the options type map for the BlockBasedTableOptions.
-  // This registration allows the Configurable class to process the option
-  // values associated with the BlockBasedTableOptions without further code in
-  // the derived class.
-  //
-  // @param name    The name of this set of options (@see GetOptionsPtr)
-  // @param opt_ptr Pointer to the options to associate with this name
-  // @param opt_map Options map that controls how this option is configured.
-  template <typename T>
-  static void RegisterOptions(
-      Configurable& configurable, T* opt_ptr,
-      const std::unordered_map<std::string, OptionTypeInfo>* opt_map) {
-    RegisterOptions(configurable, T::kName(), opt_ptr, opt_map);
-  }
-  static void RegisterOptions(
-      Configurable& configurable, const std::string& name, void* opt_ptr,
-      const std::unordered_map<std::string, OptionTypeInfo>* opt_map);
-
   // Configures the input Configurable object based on the parameters.
   // On successful completion, the Configurable is updated with the settings
   // from the opt_map.
diff --git a/options/configurable_test.cc b/options/configurable_test.cc
index 68fcf963f..5983e2dc6 100644
--- a/options/configurable_test.cc
+++ b/options/configurable_test.cc
@@ -78,18 +78,15 @@ class SimpleConfigurable : public TestConfigurable<Configurable> {
       : TestConfigurable(name, mode, map) {
     if ((mode & TestConfigMode::kUniqueMode) != 0) {
       unique_.reset(SimpleConfigurable::Create("Unique" + name_));
-      ConfigurableHelper::RegisterOptions(*this, name_ + "Unique", &unique_,
-                                          &unique_option_info);
+      RegisterOptions(name_ + "Unique", &unique_, &unique_option_info);
     }
     if ((mode & TestConfigMode::kSharedMode) != 0) {
       shared_.reset(SimpleConfigurable::Create("Shared" + name_));
-      ConfigurableHelper::RegisterOptions(*this, name_ + "Shared", &shared_,
-                                          &shared_option_info);
+      RegisterOptions(name_ + "Shared", &shared_, &shared_option_info);
     }
     if ((mode & TestConfigMode::kRawPtrMode) != 0) {
       pointer_ = SimpleConfigurable::Create("Pointer" + name_);
-      ConfigurableHelper::RegisterOptions(*this, name_ + "Pointer", &pointer_,
-                                          &pointer_option_info);
+      RegisterOptions(name_ + "Pointer", &pointer_, &pointer_option_info);
     }
   }
 
@@ -250,19 +247,15 @@ class ValidatedConfigurable : public SimpleConfigurable {
       : SimpleConfigurable(name, TestConfigMode::kDefaultMode),
         validated(false),
         prepared(0) {
-    ConfigurableHelper::RegisterOptions(*this, "Validated", &validated,
-                                        &validated_option_info);
-    ConfigurableHelper::RegisterOptions(*this, "Prepared", &prepared,
-                                        &prepared_option_info);
+    RegisterOptions("Validated", &validated, &validated_option_info);
+    RegisterOptions("Prepared", &prepared, &prepared_option_info);
     if ((mode & TestConfigMode::kUniqueMode) != 0) {
       unique_.reset(new ValidatedConfigurable(
           "Unique" + name_, TestConfigMode::kDefaultMode, false));
       if (dont_prepare) {
-        ConfigurableHelper::RegisterOptions(*this, name_ + "Unique", &unique_,
-                                            &dont_prepare_option_info);
+        RegisterOptions(name_ + "Unique", &unique_, &dont_prepare_option_info);
       } else {
-        ConfigurableHelper::RegisterOptions(*this, name_ + "Unique", &unique_,
-                                            &unique_option_info);
+        RegisterOptions(name_ + "Unique", &unique_, &unique_option_info);
       }
     }
   }
@@ -353,10 +346,8 @@ TEST_F(ConfigurableTest, MutableOptionsTest) {
         : SimpleConfigurable("mutable", TestConfigMode::kDefaultMode |
                                             TestConfigMode::kUniqueMode |
                                             TestConfigMode::kSharedMode) {
-      ConfigurableHelper::RegisterOptions(*this, "struct", &options_,
-                                          &struct_option_info);
-      ConfigurableHelper::RegisterOptions(*this, "imm", &options_,
-                                          &imm_option_info);
+      RegisterOptions("struct", &options_, &struct_option_info);
+      RegisterOptions("imm", &options_, &imm_option_info);
     }
   };
   MutableConfigurable mc;
diff --git a/options/configurable_test.h b/options/configurable_test.h
index 52c3599f6..cf9d06678 100644
--- a/options/configurable_test.h
+++ b/options/configurable_test.h
@@ -112,11 +112,10 @@ class TestConfigurable : public Configurable {
       : name_(name), pointer_(nullptr) {
     prefix_ = "test." + name + ".";
     if ((mode & TestConfigMode::kSimpleMode) != 0) {
-      ConfigurableHelper::RegisterOptions(*this, name_, &options_, map);
+      RegisterOptions(name_, &options_, map);
     }
     if ((mode & TestConfigMode::kEnumMode) != 0) {
-      ConfigurableHelper::RegisterOptions(*this, name_ + "Enum", &options_,
-                                          &enum_option_info);
+      RegisterOptions(name_ + "Enum", &options_, &enum_option_info);
     }
   }
 
diff --git a/options/customizable_helper.h b/options/customizable_helper.h
index ff07e5498..2a2c12b62 100644
--- a/options/customizable_helper.h
+++ b/options/customizable_helper.h
@@ -9,7 +9,6 @@
 #include <unordered_map>
 
 #include "options/configurable_helper.h"
-#include "options/options_helper.h"
 #include "rocksdb/convenience.h"
 #include "rocksdb/customizable.h"
 #include "rocksdb/status.h"
diff --git a/options/customizable_test.cc b/options/customizable_test.cc
index 4726e4866..48e0ab244 100644
--- a/options/customizable_test.cc
+++ b/options/customizable_test.cc
@@ -98,8 +98,9 @@ static std::unordered_map<std::string, OptionTypeInfo> a_option_info = {
 };
 class ACustomizable : public TestCustomizable {
  public:
-  ACustomizable(const std::string& id) : TestCustomizable("A"), id_(id) {
-    ConfigurableHelper::RegisterOptions(*this, "A", &opts_, &a_option_info);
+  explicit ACustomizable(const std::string& id)
+      : TestCustomizable("A"), id_(id) {
+    RegisterOptions("A", &opts_, &a_option_info);
   }
   std::string GetId() const override { return id_; }
   static const char* kClassName() { return "A"; }
@@ -141,8 +142,8 @@ static std::unordered_map<std::string, OptionTypeInfo> b_option_info = {
 class BCustomizable : public TestCustomizable {
  private:
  public:
-  BCustomizable(const std::string& name) : TestCustomizable(name) {
-    ConfigurableHelper::RegisterOptions(*this, name, &opts_, &b_option_info);
+  explicit BCustomizable(const std::string& name) : TestCustomizable(name) {
+    RegisterOptions(name, &opts_, &b_option_info);
   }
   static const char* kClassName() { return "B"; }
 
@@ -246,13 +247,12 @@ class SimpleConfigurable : public Configurable {
 
  public:
   SimpleConfigurable() {
-    ConfigurableHelper::RegisterOptions(*this, "simple", &simple_,
-                                        &simple_option_info);
+    RegisterOptions("simple", &simple_, &simple_option_info);
   }
 
-  SimpleConfigurable(
+  explicit SimpleConfigurable(
       const std::unordered_map<std::string, OptionTypeInfo>* map) {
-    ConfigurableHelper::RegisterOptions(*this, "simple", &simple_, map);
+    RegisterOptions("simple", &simple_, map);
   }
 
   bool IsPrepared() const override {
@@ -509,8 +509,7 @@ class ShallowCustomizable : public Customizable {
  public:
   ShallowCustomizable() {
     inner_ = std::make_shared<ACustomizable>("a");
-    ConfigurableHelper::RegisterOptions(*this, "inner", &inner_,
-                                        &inner_option_info);
+    RegisterOptions("inner", &inner_, &inner_option_info);
   };
   static const char* kClassName() { return "shallow"; }
   const char* Name() const override { return kClassName(); }
@@ -641,10 +640,8 @@ TEST_F(CustomizableTest, MutableOptionsTest) {
 
    public:
     MutableCustomizable() {
-      ConfigurableHelper::RegisterOptions(*this, "mutable", &mutable_,
-                                          &mutable_option_info);
-      ConfigurableHelper::RegisterOptions(*this, "immutable", &immutable_,
-                                          &immutable_option_info);
+      RegisterOptions("mutable", &mutable_, &mutable_option_info);
+      RegisterOptions("immutable", &immutable_, &immutable_option_info);
     }
     const char* Name() const override { return "MutableCustomizable"; }
   };
diff --git a/options/db_options.cc b/options/db_options.cc
index af88e41d4..ca1414758 100644
--- a/options/db_options.cc
+++ b/options/db_options.cc
@@ -435,10 +435,9 @@ const std::string OptionsHelper::kDBOptionsName = "DBOptions";
 
 class MutableDBConfigurable : public Configurable {
  public:
-  MutableDBConfigurable(const MutableDBOptions& mdb) {
+  explicit MutableDBConfigurable(const MutableDBOptions& mdb) {
     mutable_ = mdb;
-    ConfigurableHelper::RegisterOptions(*this, &mutable_,
-                                        &db_mutable_options_type_info);
+    RegisterOptions(&mutable_, &db_mutable_options_type_info);
   }
 
  protected:
@@ -447,7 +446,7 @@ class MutableDBConfigurable : public Configurable {
 
 class DBOptionsConfigurable : public MutableDBConfigurable {
  public:
-  DBOptionsConfigurable(const DBOptions& opts)
+  explicit DBOptionsConfigurable(const DBOptions& opts)
       : MutableDBConfigurable(MutableDBOptions(opts)), db_options_(opts) {
     // The ImmutableDBOptions currently requires the env to be non-null.  Make
     // sure it is
@@ -458,8 +457,7 @@ class DBOptionsConfigurable : public MutableDBConfigurable {
       copy.env = Env::Default();
       immutable_ = ImmutableDBOptions(copy);
     }
-    ConfigurableHelper::RegisterOptions(*this, &immutable_,
-                                        &db_immutable_options_type_info);
+    RegisterOptions(&immutable_, &db_immutable_options_type_info);
   }
 
  protected:
diff --git a/table/block_based/block_based_table_factory.cc b/table/block_based/block_based_table_factory.cc
index d7ee41dc9..e9fb137f1 100644
--- a/table/block_based/block_based_table_factory.cc
+++ b/table/block_based/block_based_table_factory.cc
@@ -428,8 +428,7 @@ BlockBasedTableFactory::BlockBasedTableFactory(
     const BlockBasedTableOptions& _table_options)
     : table_options_(_table_options) {
   InitializeOptions();
-  ConfigurableHelper::RegisterOptions(*this, &table_options_,
-                                      &block_based_table_type_info);
+  RegisterOptions(&table_options_, &block_based_table_type_info);
 }
 
 void BlockBasedTableFactory::InitializeOptions() {
diff --git a/table/cuckoo/cuckoo_table_factory.cc b/table/cuckoo/cuckoo_table_factory.cc
index c6d3c377c..5fe9ef749 100644
--- a/table/cuckoo/cuckoo_table_factory.cc
+++ b/table/cuckoo/cuckoo_table_factory.cc
@@ -95,8 +95,7 @@ static std::unordered_map<std::string, OptionTypeInfo> cuckoo_table_type_info =
 
 CuckooTableFactory::CuckooTableFactory(const CuckooTableOptions& table_options)
     : table_options_(table_options) {
-  ConfigurableHelper::RegisterOptions(*this, &table_options_,
-                                      &cuckoo_table_type_info);
+  RegisterOptions(&table_options_, &cuckoo_table_type_info);
 }
 
 TableFactory* NewCuckooTableFactory(const CuckooTableOptions& table_options) {
diff --git a/table/plain/plain_table_factory.cc b/table/plain/plain_table_factory.cc
index e0d0e69f6..67b4bd3e5 100644
--- a/table/plain/plain_table_factory.cc
+++ b/table/plain/plain_table_factory.cc
@@ -52,8 +52,7 @@ static std::unordered_map<std::string, OptionTypeInfo> plain_table_type_info = {
 
 PlainTableFactory::PlainTableFactory(const PlainTableOptions& options)
     : table_options_(options) {
-  ConfigurableHelper::RegisterOptions(*this, &table_options_,
-                                      &plain_table_type_info);
+  RegisterOptions(&table_options_, &plain_table_type_info);
 }
 
 Status PlainTableFactory::NewTableReader(