Better code: remove unnecessary usage of const_cast and std::function.

This commit is contained in:
Vitaly Baranov 2024-03-18 16:44:55 +01:00
parent 306f642d97
commit c8375cd167
18 changed files with 41 additions and 68 deletions

View File

@ -98,7 +98,7 @@ public:
bool supportUpdates() const override { return false; }
std::shared_ptr<const IExternalLoadable> clone() const override
std::shared_ptr<IExternalLoadable> clone() const override
{
return std::make_shared<CacheDictionary>(
getDictionaryID(),

View File

@ -50,7 +50,7 @@ public:
double getLoadFactor() const override { return 0; }
std::shared_ptr<const IExternalLoadable> clone() const override
std::shared_ptr<IExternalLoadable> clone() const override
{
return std::make_shared<DirectDictionary>(getDictionaryID(), dict_struct, source_ptr->clone());
}

View File

@ -57,7 +57,7 @@ public:
double getLoadFactor() const override { return static_cast<double>(element_count) / bucket_count; }
std::shared_ptr<const IExternalLoadable> clone() const override
std::shared_ptr<IExternalLoadable> clone() const override
{
return std::make_shared<FlatDictionary>(getDictionaryID(), dict_struct, source_ptr->clone(), configuration, update_field_loaded_block);
}

View File

@ -73,7 +73,7 @@ public:
double getLoadFactor() const override { return static_cast<double>(total_element_count) / bucket_count; }
std::shared_ptr<const IExternalLoadable> clone() const override
std::shared_ptr<IExternalLoadable> clone() const override
{
return std::make_shared<HashedArrayDictionary<dictionary_key_type, sharded>>(getDictionaryID(), dict_struct, source_ptr->clone(), configuration, update_field_loaded_block);
}

View File

@ -116,7 +116,7 @@ public:
double getLoadFactor() const override { return static_cast<double>(element_count) / bucket_count; }
std::shared_ptr<const IExternalLoadable> clone() const override
std::shared_ptr<IExternalLoadable> clone() const override
{
return std::make_shared<HashedDictionary<dictionary_key_type, sparse, sharded>>(
getDictionaryID(),

View File

@ -57,7 +57,7 @@ public:
double getLoadFactor() const override { return static_cast<double>(element_count) / bucket_count; }
std::shared_ptr<const IExternalLoadable> clone() const override
std::shared_ptr<IExternalLoadable> clone() const override
{
return std::make_shared<IPAddressDictionary>(getDictionaryID(), dict_struct, source_ptr->clone(), configuration);
}

View File

@ -29,7 +29,7 @@ PolygonDictionarySimple::PolygonDictionarySimple(
{
}
std::shared_ptr<const IExternalLoadable> PolygonDictionarySimple::clone() const
std::shared_ptr<IExternalLoadable> PolygonDictionarySimple::clone() const
{
return std::make_shared<PolygonDictionarySimple>(
this->getDictionaryID(),
@ -76,7 +76,7 @@ PolygonDictionaryIndexEach::PolygonDictionaryIndexEach(
}
}
std::shared_ptr<const IExternalLoadable> PolygonDictionaryIndexEach::clone() const
std::shared_ptr<IExternalLoadable> PolygonDictionaryIndexEach::clone() const
{
return std::make_shared<PolygonDictionaryIndexEach>(
this->getDictionaryID(),
@ -126,7 +126,7 @@ PolygonDictionaryIndexCell::PolygonDictionaryIndexCell(
{
}
std::shared_ptr<const IExternalLoadable> PolygonDictionaryIndexCell::clone() const
std::shared_ptr<IExternalLoadable> PolygonDictionaryIndexCell::clone() const
{
return std::make_shared<PolygonDictionaryIndexCell>(
this->getDictionaryID(),

View File

@ -23,7 +23,7 @@ public:
DictionaryLifetime dict_lifetime_,
Configuration configuration_);
std::shared_ptr<const IExternalLoadable> clone() const override;
std::shared_ptr<IExternalLoadable> clone() const override;
private:
bool find(const Point & point, size_t & polygon_index) const override;
@ -47,7 +47,7 @@ public:
int min_intersections_,
int max_depth_);
std::shared_ptr<const IExternalLoadable> clone() const override;
std::shared_ptr<IExternalLoadable> clone() const override;
static constexpr size_t kMinIntersectionsDefault = 1;
static constexpr size_t kMaxDepthDefault = 5;
@ -75,7 +75,7 @@ public:
size_t min_intersections_,
size_t max_depth_);
std::shared_ptr<const IExternalLoadable> clone() const override;
std::shared_ptr<IExternalLoadable> clone() const override;
static constexpr size_t kMinIntersectionsDefault = 1;
static constexpr size_t kMaxDepthDefault = 5;

View File

@ -101,7 +101,7 @@ public:
double getLoadFactor() const override { return static_cast<double>(element_count) / bucket_count; }
std::shared_ptr<const IExternalLoadable> clone() const override
std::shared_ptr<IExternalLoadable> clone() const override
{
auto result = std::make_shared<RangeHashedDictionary>(
getDictionaryID(),

View File

@ -86,7 +86,7 @@ public:
bool hasHierarchy() const override { return false; }
std::shared_ptr<const IExternalLoadable> clone() const override
std::shared_ptr<IExternalLoadable> clone() const override
{
return std::make_shared<RegExpTreeDictionary>(
getDictionaryID(), structure, source_ptr->clone(), configuration, use_vectorscan, flag_case_insensitive, flag_dotall);

View File

@ -120,7 +120,7 @@ void ExternalUserDefinedExecutableFunctionsLoader::reloadFunction(const std::str
loadOrReload(user_defined_function_name);
}
ExternalLoader::LoadablePtr ExternalUserDefinedExecutableFunctionsLoader::create(const std::string & name,
ExternalLoader::LoadableMutablePtr ExternalUserDefinedExecutableFunctionsLoader::createObject(const std::string & name,
const Poco::Util::AbstractConfiguration & config,
const std::string & key_in_config,
const std::string &) const

View File

@ -28,7 +28,7 @@ public:
void reloadFunction(const std::string & user_defined_function_name) const;
protected:
LoadablePtr create(const std::string & name,
LoadableMutablePtr createObject(const std::string & name,
const Poco::Util::AbstractConfiguration & config,
const std::string & key_in_config,
const std::string & repository_name) const override;

View File

@ -62,7 +62,7 @@ public:
return true;
}
std::shared_ptr<const IExternalLoadable> clone() const override
std::shared_ptr<IExternalLoadable> clone() const override
{
return std::make_shared<UserDefinedExecutableFunction>(configuration, coordinator, lifetime);
}

View File

@ -32,7 +32,7 @@ ExternalDictionariesLoader::ExternalDictionariesLoader(ContextPtr global_context
enablePeriodicUpdates(true);
}
ExternalLoader::LoadablePtr ExternalDictionariesLoader::create(
ExternalLoader::LoadableMutablePtr ExternalDictionariesLoader::createObject(
const std::string & name, const Poco::Util::AbstractConfiguration & config,
const std::string & key_in_config, const std::string & repository_name) const
{

View File

@ -40,8 +40,8 @@ public:
static void resetAll();
protected:
LoadablePtr create(const std::string & name, const Poco::Util::AbstractConfiguration & config,
const std::string & key_in_config, const std::string & repository_name) const override;
LoadableMutablePtr createObject(const std::string & name, const Poco::Util::AbstractConfiguration & config,
const std::string & key_in_config, const std::string & repository_name) const override;
bool doesConfigChangeRequiresReloadingObject(const Poco::Util::AbstractConfiguration & old_config, const String & old_key_in_config,
const Poco::Util::AbstractConfiguration & new_config, const String & new_key_in_config) const override;

View File

@ -87,9 +87,6 @@ namespace
lock = std::unique_lock(mutex);
}
};
using DoesConfigChangeRequiresReloadingObjectFunction = std::function<bool(const Poco::Util::AbstractConfiguration & config_1, const String & key_in_config_1, const Poco::Util::AbstractConfiguration & config_2, const String & key_in_config_2)>;
using UpdateObjectFromConfigWithoutReloadingFunction = std::function<void(const IExternalLoadable & object, const Poco::Util::AbstractConfiguration & config, const String & key_in_config)>;
}
@ -98,10 +95,7 @@ namespace
class ExternalLoader::LoadablesConfigReader : private boost::noncopyable
{
public:
LoadablesConfigReader(const String & type_name_, LoggerPtr log_)
: type_name(type_name_), log(log_)
{
}
LoadablesConfigReader(const String & type_name_, LoggerPtr log_) : type_name(type_name_), log(log_) { }
~LoadablesConfigReader() = default;
using Repository = IExternalLoaderConfigRepository;
@ -397,21 +391,8 @@ private:
class ExternalLoader::LoadingDispatcher : private boost::noncopyable
{
public:
/// Called to load or reload an object.
using CreateObjectFunction = std::function<LoadablePtr(
const String & /* name */, const ObjectConfig & /* config */, const LoadablePtr & /* previous_version */)>;
LoadingDispatcher(
const CreateObjectFunction & create_object_function_,
const DoesConfigChangeRequiresReloadingObjectFunction & does_config_change_requires_reloading_object_,
const UpdateObjectFromConfigWithoutReloadingFunction & update_object_from_config_without_reloading_,
const String & type_name_,
LoggerPtr log_)
: create_object(create_object_function_)
, does_config_change_requires_reloading_object(does_config_change_requires_reloading_object_)
, update_object_from_config_without_reloading(update_object_from_config_without_reloading_)
, type_name(type_name_)
, log(log_)
LoadingDispatcher(const String & type_name_, LoggerPtr log_, const ExternalLoader & external_loader_)
: type_name(type_name_), log(log_), external_loader(external_loader_)
{
}
@ -471,11 +452,11 @@ public:
if (config_changed)
{
if (info.object)
update_object_from_config_without_reloading(*info.object, *new_config->config, new_config->key_in_config);
external_loader.updateObjectFromConfigWithoutReloading(*info.object, *new_config->config, new_config->key_in_config);
if (info.triedToLoad())
{
bool config_change_requires_reloading = does_config_change_requires_reloading_object(*previous_config->config, previous_config->key_in_config, *new_config->config, new_config->key_in_config);
bool config_change_requires_reloading = external_loader.doesConfigChangeRequiresReloadingObject(*previous_config->config, previous_config->key_in_config, *new_config->config, new_config->key_in_config);
if (config_change_requires_reloading)
{
/// The object has been tried to load before, so it is currently in use or was in use
@ -786,7 +767,7 @@ private:
}
String name;
LoadablePtr object;
LoadableMutablePtr object;
std::shared_ptr<const ObjectConfig> config;
TimePoint loading_start_time;
TimePoint loading_end_time;
@ -1046,17 +1027,17 @@ private:
}
/// Load one object, returns object ptr or exception.
std::pair<LoadablePtr, std::exception_ptr>
std::pair<LoadableMutablePtr, std::exception_ptr>
loadSingleObject(const String & name, const ObjectConfig & config, LoadablePtr previous_version)
{
/// Use `create_function` to perform the actual loading.
/// It's much better to do it with `mutex` unlocked because the loading can take a lot of time
/// and require access to other objects.
LoadablePtr new_object;
LoadableMutablePtr new_object;
std::exception_ptr new_exception;
try
{
new_object = create_object(name, config, previous_version);
new_object = external_loader.createOrCloneObject(name, config, previous_version);
}
catch (...)
{
@ -1070,7 +1051,7 @@ private:
const String & name,
size_t loading_id,
LoadablePtr previous_version,
LoadablePtr new_object,
LoadableMutablePtr new_object,
std::exception_ptr new_exception,
size_t error_count,
const LoadingGuardForAsyncLoad &)
@ -1134,7 +1115,7 @@ private:
if (new_object)
{
update_object_from_config_without_reloading(*new_object, *info->config->config, info->config->key_in_config);
external_loader.updateObjectFromConfigWithoutReloading(*new_object, *info->config->config, info->config->key_in_config);
info->object = new_object;
}
@ -1210,11 +1191,9 @@ private:
}
}
const CreateObjectFunction create_object;
const DoesConfigChangeRequiresReloadingObjectFunction does_config_change_requires_reloading_object;
const UpdateObjectFromConfigWithoutReloadingFunction update_object_from_config_without_reloading;
const String type_name;
LoggerPtr log;
const LoggerPtr log;
const ExternalLoader & external_loader;
mutable std::mutex mutex;
std::condition_variable event;
@ -1296,14 +1275,7 @@ private:
ExternalLoader::ExternalLoader(const String & type_name_, LoggerPtr log_)
: config_files_reader(std::make_unique<LoadablesConfigReader>(type_name_, log_))
, loading_dispatcher(std::make_unique<LoadingDispatcher>(
[this](auto && a, auto && b, auto && c) { return createObject(a, b, c); },
[this](const Poco::Util::AbstractConfiguration & config_1, const String & key_in_config_1, const Poco::Util::AbstractConfiguration & config_2, const String & key_in_config_2)
{ return doesConfigChangeRequiresReloadingObject(config_1, key_in_config_1, config_2, key_in_config_2); },
[this](const IExternalLoadable & object, const Poco::Util::AbstractConfiguration & config, const String & key_in_config)
{ return updateObjectFromConfigWithoutReloading(const_cast<IExternalLoadable &>(object), config, key_in_config); },
type_name_,
log_))
, loading_dispatcher(std::make_unique<LoadingDispatcher>(type_name_, log_, *this))
, periodic_updater(std::make_unique<PeriodicUpdater>(*config_files_reader, *loading_dispatcher))
, type_name(type_name_)
, log(log_)
@ -1530,13 +1502,13 @@ void ExternalLoader::reloadConfig(const String & repository_name, const String &
loading_dispatcher->setConfiguration(config_files_reader->read(repository_name, path));
}
ExternalLoader::LoadablePtr ExternalLoader::createObject(
ExternalLoader::LoadableMutablePtr ExternalLoader::createOrCloneObject(
const String & name, const ObjectConfig & config, const LoadablePtr & previous_version) const
{
if (previous_version)
return previous_version->clone();
return create(name, *config.config, config.key_in_config, config.repository_name);
return createObject(name, *config.config, config.key_in_config, config.repository_name);
}
template ExternalLoader::LoadablePtr ExternalLoader::getLoadResult<ExternalLoader::LoadablePtr>(const String &) const;

View File

@ -50,6 +50,7 @@ class ExternalLoader
{
public:
using LoadablePtr = std::shared_ptr<const IExternalLoadable>;
using LoadableMutablePtr = std::shared_ptr<IExternalLoadable>;
using Loadables = std::vector<LoadablePtr>;
using Status = ExternalLoaderStatus;
@ -211,7 +212,7 @@ public:
void reloadConfig(const String & repository_name, const String & path) const;
protected:
virtual LoadablePtr create(const String & name, const Poco::Util::AbstractConfiguration & config, const String & key_in_config, const String & repository_name) const = 0;
virtual LoadableMutablePtr createObject(const String & name, const Poco::Util::AbstractConfiguration & config, const String & key_in_config, const String & repository_name) const = 0;
/// Returns whether the object must be reloaded after a specified change in its configuration.
virtual bool doesConfigChangeRequiresReloadingObject(const Poco::Util::AbstractConfiguration & /* old_config */, const String & /* old_key_in_config */,
@ -227,7 +228,7 @@ private:
Strings getAllTriedToLoadNames() const;
LoadablePtr createObject(const String & name, const ObjectConfig & config, const LoadablePtr & previous_version) const;
LoadableMutablePtr createOrCloneObject(const String & name, const ObjectConfig & config, const LoadablePtr & previous_version) const;
class LoadablesConfigReader;
std::unique_ptr<LoadablesConfigReader> config_files_reader;
@ -239,7 +240,7 @@ private:
std::unique_ptr<PeriodicUpdater> periodic_updater;
const String type_name;
LoggerPtr log;
const LoggerPtr log;
};
}

View File

@ -43,7 +43,7 @@ public:
/// If lifetime exceeded and isModified(), ExternalLoader replace current object with the result of clone().
virtual bool isModified() const = 0;
/// Returns new object with the same configuration. Is used to update modified object when lifetime exceeded.
virtual std::shared_ptr<const IExternalLoadable> clone() const = 0;
virtual std::shared_ptr<IExternalLoadable> clone() const = 0;
};
}