Use an HashMap to store options in option manager

Turns out looking for options can get pretty slow, so O(1) lookup
seems better.

This should improve the performance of the #1460 issue
This commit is contained in:
Maxime Coste 2017-06-23 09:54:21 +01:00
parent fb22349249
commit 7ceb768a2e
3 changed files with 21 additions and 27 deletions

View File

@ -48,13 +48,13 @@ struct option_not_found : public runtime_error
Option& OptionManager::get_local_option(StringView name) Option& OptionManager::get_local_option(StringView name)
{ {
auto it = find_option(m_options, name); auto it = m_options.find(name);
if (it != m_options.end()) if (it != m_options.end())
return **it; return *(it->value);
else if (m_parent) else if (m_parent)
{ {
m_options.emplace_back((*m_parent)[name].clone(*this)); auto* clone = (*m_parent)[name].clone(*this);
return *m_options.back(); return *m_options.insert({clone->name(), std::unique_ptr<Option>{clone}});
} }
else else
throw option_not_found(name); throw option_not_found(name);
@ -63,9 +63,9 @@ Option& OptionManager::get_local_option(StringView name)
Option& OptionManager::operator[](StringView name) Option& OptionManager::operator[](StringView name)
{ {
auto it = find_option(m_options, name); auto it = m_options.find(name);
if (it != m_options.end()) if (it != m_options.end())
return **it; return *it->value;
else if (m_parent) else if (m_parent)
return (*m_parent)[name]; return (*m_parent)[name];
else else
@ -80,10 +80,9 @@ const Option& OptionManager::operator[](StringView name) const
void OptionManager::unset_option(StringView name) void OptionManager::unset_option(StringView name)
{ {
kak_assert(m_parent); // cannot unset option on global manager kak_assert(m_parent); // cannot unset option on global manager
auto it = find_option(m_options, name); if (m_options.contains(name))
if (it != m_options.end())
{ {
m_options.erase(it); m_options.erase(name);
on_option_changed((*m_parent)[name]); on_option_changed((*m_parent)[name]);
} }
} }
@ -93,11 +92,11 @@ OptionManager::OptionList OptionManager::flatten_options() const
OptionList res = m_parent ? m_parent->flatten_options() : OptionList{}; OptionList res = m_parent ? m_parent->flatten_options() : OptionList{};
for (auto& option : m_options) for (auto& option : m_options)
{ {
auto it = find_option(res, option->name()); auto it = find_if(res, [&](const Option* opt) { return opt->name() == option.key; });
if (it != res.end()) if (it != res.end())
*it = option.get(); *it = option.value.get();
else else
res.emplace_back(option.get()); res.emplace_back(option.value.get());
} }
return res; return res;
} }
@ -105,8 +104,7 @@ OptionManager::OptionList OptionManager::flatten_options() const
void OptionManager::on_option_changed(const Option& option) void OptionManager::on_option_changed(const Option& option)
{ {
// if parent option changed, but we overrided it, it's like nothing happened // if parent option changed, but we overrided it, it's like nothing happened
if (&option.manager() != this and if (&option.manager() != this and m_options.contains(option.name()))
find_option(m_options, option.name()) != m_options.end())
return; return;
// The watcher list might get mutated during calls to on_option_changed // The watcher list might get mutated during calls to on_option_changed

View File

@ -6,6 +6,8 @@
#include "exception.hh" #include "exception.hh"
#include "option.hh" #include "option.hh"
#include "vector.hh" #include "vector.hh"
#include "hash_map.hh"
#include "utils.hh"
#include <memory> #include <memory>
#include <type_traits> #include <type_traits>
@ -101,7 +103,7 @@ private:
friend class Scope; friend class Scope;
friend class OptionsRegistry; friend class OptionsRegistry;
Vector<std::unique_ptr<Option>, MemoryDomain::Options> m_options; HashMap<StringView, std::unique_ptr<Option>, MemoryDomain::Options> m_options;
OptionManager* m_parent; OptionManager* m_parent;
mutable Vector<OptionManagerWatcher*, MemoryDomain::Options> m_watchers; mutable Vector<OptionManagerWatcher*, MemoryDomain::Options> m_watchers;
@ -190,13 +192,6 @@ template<typename T> bool Option::is_of_type() const
return dynamic_cast<const TypedOption<T>*>(this) != nullptr; return dynamic_cast<const TypedOption<T>*>(this) != nullptr;
} }
template<typename T>
auto find_option(T& container, StringView name) -> decltype(container.begin())
{
using ptr_type = decltype(*container.begin());
return find_if(container, [&name](const ptr_type& opt) { return opt->name() == name; });
}
class OptionsRegistry class OptionsRegistry
{ {
public: public:
@ -217,18 +212,18 @@ public:
throw runtime_error{format("name '{}' contains char out of [a-zA-Z0-9_]", name)}; throw runtime_error{format("name '{}' contains char out of [a-zA-Z0-9_]", name)};
auto& opts = m_global_manager.m_options; auto& opts = m_global_manager.m_options;
auto it = find_option(opts, name); auto it = opts.find(name);
if (it != opts.end()) if (it != opts.end())
{ {
if ((*it)->is_of_type<T>() and (*it)->flags() == flags) if (it->value->is_of_type<T>() and it->value->flags() == flags)
return **it; return *it->value;
throw runtime_error{format("option '{}' already declared with different type or flags", name)}; throw runtime_error{format("option '{}' already declared with different type or flags", name)};
} }
String doc = docstring.empty() ? format("[{}]", option_type_name(Meta::Type<T>{})) String doc = docstring.empty() ? format("[{}]", option_type_name(Meta::Type<T>{}))
: format("[{}] - {}", option_type_name(Meta::Type<T>{}), docstring); : format("[{}] - {}", option_type_name(Meta::Type<T>{}), docstring);
m_descs.emplace_back(new OptionDesc{name.str(), std::move(doc), flags}); m_descs.emplace_back(new OptionDesc{name.str(), std::move(doc), flags});
opts.emplace_back(new TypedCheckedOption<T, validator>{m_global_manager, *m_descs.back(), value}); return *opts.insert({m_descs.back()->name(),
return *opts.back(); make_unique<TypedCheckedOption<T, validator>>(m_global_manager, *m_descs.back(), value)});
} }
const OptionDesc* option_desc(StringView name) const const OptionDesc* option_desc(StringView name) const

View File

@ -257,6 +257,7 @@ private:
static_assert(std::is_trivial<StringView>::value, ""); static_assert(std::is_trivial<StringView>::value, "");
template<> struct HashCompatible<String, StringView> : std::true_type {}; template<> struct HashCompatible<String, StringView> : std::true_type {};
template<> struct HashCompatible<StringView, String> : std::true_type {};
inline String::String(StringView str) : String{str.begin(), str.length()} {} inline String::String(StringView str) : String{str.begin(), str.length()} {}