Avoid expensive copies of Hooks in HookManager::run_hooks

Use a deferred deletion mechanism to ensure hooks are kept alive
for the duration of run_hooks.
This commit is contained in:
Maxime Coste 2017-06-07 12:15:16 +01:00
parent 87477cf2bb
commit 4606453fed
3 changed files with 38 additions and 26 deletions

View File

@ -810,8 +810,9 @@ const CommandDesc add_hook_cmd = {
ScopedSetBool disable_history{context.history_disabled()};
MatchResults<const char*> res;
if (regex_match(param.begin(), param.end(), res, regex))
{
if (not regex_match(param.begin(), param.end(), res, regex))
return;
EnvVarMap env_vars{ {"hook_param", param.str()} };
for (size_t i = 0; i < res.size(); ++i)
env_vars.insert({format("hook_param_capture_{}", i),
@ -819,7 +820,6 @@ const CommandDesc add_hook_cmd = {
CommandManager::instance().execute(command, context,
{ {}, std::move(env_vars) });
}
};
auto group = parser.get_switch("group").value_or(StringView{});
get_scope(parser[0], context).hooks().add_hook(parser[1], group.str(), std::move(hook_func));

View File

@ -15,7 +15,7 @@ namespace Kakoune
void HookManager::add_hook(StringView hook_name, String group, HookFunc func)
{
auto& hooks = m_hooks[hook_name];
hooks.push_back({std::move(group), std::move(func)});
hooks.emplace_back(new Hook{std::move(group), std::move(func)});
}
void HookManager::remove_hooks(StringView group)
@ -23,9 +23,15 @@ void HookManager::remove_hooks(StringView group)
if (group.empty())
throw runtime_error("invalid id");
for (auto& list : m_hooks)
list.value.erase(std::remove_if(list.value.begin(), list.value.end(),
[&](const Hook& h) { return h.group == group; }),
list.value.end());
{
auto it = std::remove_if(list.value.begin(), list.value.end(),
[&](const std::unique_ptr<Hook>& h)
{ return h->group == group; });
if (not m_running_hooks.empty()) // we are running some hooks, defer deletion
m_hooks_trash.insert(m_hooks_trash.end(), std::make_move_iterator(it),
std::make_move_iterator(list.value.end()));
list.value.erase(it, list.value.end());
}
}
CandidateList HookManager::complete_hook_group(StringView prefix, ByteCount pos_in_token)
@ -33,7 +39,7 @@ CandidateList HookManager::complete_hook_group(StringView prefix, ByteCount pos_
CandidateList res;
for (auto& list : m_hooks)
{
auto container = list.value | transform(std::mem_fn(&decltype(list.value)::value_type::group));
auto container = list.value | transform([](const std::unique_ptr<Hook>& h) -> const String& { return h->group; });
for (auto& c : complete(prefix, pos_in_token, container))
{
if (!contains(res, c))
@ -52,8 +58,8 @@ void HookManager::run_hook(StringView hook_name,
if (m_parent)
m_parent->run_hook(hook_name, param, context);
auto hook_list_it = m_hooks.find(hook_name);
if (hook_list_it == m_hooks.end())
auto hook_list = m_hooks.find(hook_name);
if (hook_list == m_hooks.end())
return;
if (contains(m_running_hooks, std::make_pair(hook_name, param)))
@ -64,19 +70,23 @@ void HookManager::run_hook(StringView hook_name,
}
m_running_hooks.emplace_back(hook_name, param);
auto pop_running_hook = on_scope_end([this]{ m_running_hooks.pop_back(); });
auto pop_running_hook = on_scope_end([this]{
m_running_hooks.pop_back();
if (m_running_hooks.empty())
m_hooks_trash.clear();
});
const DebugFlags debug_flags = context.options()["debug"].get<DebugFlags>();
const bool profile = debug_flags & DebugFlags::Profile;
auto start_time = profile ? Clock::now() : TimePoint{};
auto& disabled_hooks = context.options()["disabled_hooks"].get<Regex>();
Vector<Hook> hooks_to_run;
for (auto& hook : hook_list_it->value)
Vector<Hook*> hooks_to_run; // The m_hooks_trash vector ensure hooks wont die during this method
for (auto& hook : hook_list->value)
{
if (hook.group.empty() or disabled_hooks.empty() or
not regex_match(hook.group.begin(), hook.group.end(), disabled_hooks))
hooks_to_run.push_back(hook);
if (hook->group.empty() or disabled_hooks.empty() or
not regex_match(hook->group.begin(), hook->group.end(), disabled_hooks))
hooks_to_run.push_back(hook.get());
}
bool hook_error = false;
@ -85,14 +95,14 @@ void HookManager::run_hook(StringView hook_name,
try
{
if (debug_flags & DebugFlags::Hooks)
write_to_debug_buffer(format("hook {}({})/{}", hook_name, param, hook.group));
hook.func(param, context);
write_to_debug_buffer(format("hook {}({})/{}", hook_name, param, hook->group));
hook->func(param, context);
}
catch (runtime_error& err)
{
hook_error = true;
write_to_debug_buffer(format("error running hook {}({})/{}: {}",
hook_name, param, hook.group, err.what()));
hook_name, param, hook->group, err.what()));
}
}

View File

@ -35,8 +35,10 @@ private:
};
SafePtr<HookManager> m_parent;
HashMap<String, Vector<Hook, MemoryDomain::Hooks>, MemoryDomain::Hooks> m_hooks;
HashMap<String, Vector<std::unique_ptr<Hook>, MemoryDomain::Hooks>, MemoryDomain::Hooks> m_hooks;
mutable Vector<std::pair<StringView, StringView>, MemoryDomain::Hooks> m_running_hooks;
mutable Vector<std::unique_ptr<Hook>, MemoryDomain::Hooks> m_hooks_trash;
};
}