From 9efde28350a606626d58fe348fb9ad8cac5bd1bb Mon Sep 17 00:00:00 2001 From: ridiculousfish Date: Sun, 8 May 2022 15:08:28 -0700 Subject: [PATCH] Revert "Optimize exit event generation" This reverts commit 1b6ef6670f194ed850b8a32a8d259b5d86c6c1c7. This optimimzation did not carry its weight in complexity. --- src/event.cpp | 9 --------- src/event.h | 10 ---------- src/proc.cpp | 37 ++++++++++++------------------------- 3 files changed, 12 insertions(+), 44 deletions(-) diff --git a/src/event.cpp b/src/event.cpp index e82316e7c..bdb96fd19 100644 --- a/src/event.cpp +++ b/src/event.cpp @@ -224,15 +224,6 @@ event_handler_list_t event_get_function_handlers(const wcstring &name) { return result; } -enum_set_t event_get_handled_types() { - enum_set_t result{}; - auto handlers = s_event_handlers.acquire(); - for (const shared_ptr &eh : *handlers) { - result.set(eh->desc.type); - } - return result; -} - bool event_is_signal_observed(int sig) { // We are in a signal handler! Don't allocate memory, etc. bool result = false; diff --git a/src/event.h b/src/event.h index 9deb2f91d..0ec52a343 100644 --- a/src/event.h +++ b/src/event.h @@ -13,7 +13,6 @@ #include #include "common.h" -#include "enum_set.h" #include "io.h" /// The process id that is used to match any process id. @@ -38,11 +37,6 @@ enum class event_type_t { generic, }; -template <> -struct enum_info_t { - static constexpr size_t count = static_cast(event_type_t::generic) + 1; -}; - /// Null-terminated list of valid event filter names. /// These are what are valid to pass to 'functions --handlers-type' extern const wchar_t *const event_filter_names[]; @@ -138,10 +132,6 @@ void event_remove_function_handlers(const wcstring &name); /// Return all event handlers for the given function. event_handler_list_t event_get_function_handlers(const wcstring &name); -/// \return the event types for which handlers are registered. -/// This can be a performance optimization to avoid emitting events. -enum_set_t event_get_handled_types(); - /// Returns whether an event listener is registered for the given signal. This is safe to call from /// a signal handler. bool event_is_signal_observed(int signal); diff --git a/src/proc.cpp b/src/proc.cpp index 4f99675bf..929da7f99 100644 --- a/src/proc.cpp +++ b/src/proc.cpp @@ -457,28 +457,25 @@ static void process_mark_finished_children(parser_t &parser, bool block_ok) { } /// Given a job that has completed, generate job_exit, process_exit, and caller_exit events. -static void generate_exit_events(const job_ref_t &j, const enum_set_t handled, - std::vector *out_evts) { +static void generate_exit_events(const job_ref_t &j, std::vector *out_evts) { // Generate proc and job exit events, except for jobs originating in event handlers. - if (handled.get(event_type_t::process_exit) && !j->from_event_handler()) { + if (!j->from_event_handler()) { + // process_exit events. for (const auto &proc : j->processes) { if (proc->pid > 0) { out_evts->push_back(event_t::process_exit(proc->pid, proc->status.status_value())); } } - } - if (handled.get(event_type_t::job_exit) && !j->from_event_handler() && - j->posts_job_exit_events()) { - if (auto last_pid = j->get_last_pid()) { - out_evts->push_back(event_t::job_exit(*last_pid, j->internal_job_id)); + // job_exit events. + if (j->posts_job_exit_events()) { + if (auto last_pid = j->get_last_pid()) { + out_evts->push_back(event_t::job_exit(*last_pid, j->internal_job_id)); + } } } - // Generate caller_exit events. - if (handled.get(event_type_t::caller_exit)) { - out_evts->push_back(event_t::caller_exit(j->internal_job_id, j->job_id())); - } + out_evts->push_back(event_t::caller_exit(j->internal_job_id, j->job_id())); } /// \return whether to emit a fish_job_summary call for a process. @@ -658,11 +655,6 @@ static bool process_clean_after_marking(parser_t &parser, bool allow_interactive // complete. std::vector exit_events; - // Figure out which event types have handlers, so we can avoid the cost of creating events that - // are unhandled. In principle, an event handler could add a handler for another event type, - // which won't receive it. We ignore that possibility. - const enum_set_t handled_event_types = event_get_handled_types(); - // Defer processing under-construction jobs or jobs that want a message when we are not // interactive. auto should_process_job = [=](const shared_ptr &j) { @@ -696,7 +688,7 @@ static bool process_clean_after_marking(parser_t &parser, bool allow_interactive // Remember it for summary later, generate exit events, maybe save its wait handle if it // finished in the background. if (job_or_proc_wants_summary(j)) jobs_to_summarize.push_back(j); - generate_exit_events(j, handled_event_types, &exit_events); + generate_exit_events(j, &exit_events); save_wait_handle_for_completed_job(j, parser.get_wait_handles()); // Remove it. @@ -707,13 +699,8 @@ static bool process_clean_after_marking(parser_t &parser, bool allow_interactive bool printed = summarize_jobs(parser, jobs_to_summarize); // Post pending exit events. - // If we have none, still give any other pending events (e.g. from signals) a chance to run. - if (exit_events.empty()) { - event_fire_delayed(parser); - } else { - for (const auto &evt : exit_events) { - event_fire(parser, evt); - } + for (const auto &evt : exit_events) { + event_fire(parser, evt); } if (printed) {