From b1ac07a1784288e5f20caeecdaef8dcc8d413d8b Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Fri, 18 Aug 2017 17:26:45 -0700 Subject: [PATCH] Reduce overhead of setting fish vars The `react_to_variable_change()` function is called whenever a fish var is set. Even as a consequence of statements like `for x in a b c`. It is therefore critical that that function be as fast as possible. Especially when setting the var doesn't have any side-effects which is true something like 99.9999% of the time. This change reduces the overhead of `react_to_variable_change()` to unmeasurable levels. Making the synthetic benchmark in issue #4341 36% faster. Fixes #4341 --- CHANGELOG.md | 1 + src/env.cpp | 137 ++++++++++++++++++++++++++++++++++----------------- 2 files changed, 92 insertions(+), 46 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 05863074a..a205f2895 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ This section is for changes merged to the `major` branch that are not also merge - `complete` now has a `-k` and `--keep-order` option to keep the order of the OPTION_ARGUMENTS (#361). - Local exported (`set -lx`) vars are now visible to functions (#1091). - `abbr` has been reimplemented to be faster. This means the old `fish_user_abbreviations` variable is ignored (#4048). +- Setting variables is much faster meaning fish is much faster (#4200, #4341). ## Other significant changes - Command substitution output is now limited to 10 MB by default (#3822). diff --git a/src/env.cpp b/src/env.cpp index 4ab201074..356902086 100644 --- a/src/env.cpp +++ b/src/env.cpp @@ -85,6 +85,9 @@ bool term_has_xn = false; /// found in `TERMINFO_DIRS` we don't to call `handle_curses()` before we've imported the latter. static bool env_initialized = false; +typedef std::map var_dispatch_table_t; +var_dispatch_table_t var_dispatch_table; + /// List of all locale environment variable names that might trigger (re)initializing the locale /// subsystem. static const wcstring_list_t locale_variables({L"LANG", L"LANGUAGE", L"LC_ALL", L"LC_ADDRESS", @@ -348,9 +351,6 @@ static mode_t get_umask() { return res; } -/// Check if the specified variable is a timezone variable. -static bool var_is_timezone(const wcstring &key) { return key == L"TZ"; } - /// Properly sets all timezone information. static void handle_timezone(const wchar_t *env_var_name) { // const env_var_t var = env_get(env_var_name, ENV_EXPORT); @@ -399,16 +399,6 @@ static void fix_colon_delimited_var(const wcstring &var_name) { } } -/// Check if the specified variable is a locale variable. -static bool var_is_locale(const wcstring &key) { - for (auto var_name : locale_variables) { - if (key == var_name) { - return true; - } - } - return false; -} - /// Initialize the locale subsystem. static void init_locale() { // We have to make a copy because the subsequent setlocale() call to change the locale will @@ -445,16 +435,6 @@ static void init_locale() { free(old_msg_locale); } -/// Check if the specified variable is a locale variable. -static bool var_is_curses(const wcstring &key) { - for (auto var_name : curses_variables) { - if (key == var_name) { - return true; - } - } - return false; -} - /// True if we think we can set the terminal title else false. static bool can_set_term_title = false; @@ -629,33 +609,18 @@ static bool variable_is_colon_delimited_var(const wcstring &str) { static void react_to_variable_change(const wchar_t *op, const wcstring &key) { // Don't do any of this until `env_init()` has run. We only want to do this in response to // variables set by the user; e.g., in a script like *config.fish* or interactively or as part - // of loading the universal variables for the first time. + // of loading the universal variables for the first time. Variables we import from the + // environment or that are otherwise set by fish before this gets called have to explicitly + // call the appropriate functions to put the value of the var into effect. if (!env_initialized) return; - if (var_is_locale(key)) { - init_locale(); - } else if (variable_is_colon_delimited_var(key)) { - fix_colon_delimited_var(key); - } else if (var_is_curses(key)) { - init_curses(); - init_input(); - } else if (var_is_timezone(key)) { - handle_timezone(key.c_str()); - } else if (key == L"fish_term256" || key == L"fish_term24bit") { - update_fish_color_support(); - reader_react_to_color_change(); + auto dispatch = var_dispatch_table.find(key); + if (dispatch != var_dispatch_table.end()) { + (*dispatch->second)(op, key); + } else if (string_prefixes_string(L"_fish_abbr_", key)) { + update_abbr_cache(op, key); } else if (string_prefixes_string(L"fish_color_", key)) { reader_react_to_color_change(); - } else if (key == L"fish_escape_delay_ms") { - update_wait_on_escape_ms(); - } else if (key == L"LINES" || key == L"COLUMNS") { - invalidate_termsize(true); // force fish to update its idea of the terminal size plus vars - } else if (key == L"FISH_READ_BYTE_LIMIT") { - env_set_read_limit(); - } else if (key == L"FISH_HISTORY") { - reader_change_history(history_session_id().c_str()); - } else if (wcsncmp(key.c_str(), L"_fish_abbr_", wcslen(L"_fish_abbr_")) == 0) { - update_abbr_cache(op, key); } } @@ -813,7 +778,87 @@ void env_universal_barrier() { env_universal_callbacks(callbacks); } +static void handle_fish_term_change(const wcstring &op, const wcstring &var_name) { + UNUSED(op); + UNUSED(var_name); + update_fish_color_support(); + reader_react_to_color_change(); +} + +static void handle_escape_delay_change(const wcstring &op, const wcstring &var_name) { + UNUSED(op); + UNUSED(var_name); + update_wait_on_escape_ms(); +} + +static void handle_term_size_change(const wcstring &op, const wcstring &var_name) { + UNUSED(op); + UNUSED(var_name); + invalidate_termsize(true); // force fish to update its idea of the terminal size plus vars +} + +static void handle_read_limit_change(const wcstring &op, const wcstring &var_name) { + UNUSED(op); + UNUSED(var_name); + env_set_read_limit(); +} + +static void handle_fish_history_change(const wcstring &op, const wcstring &var_name) { + UNUSED(op); + UNUSED(var_name); + reader_change_history(history_session_id().c_str()); +} + +static void handle_tz_change(const wcstring &op, const wcstring &var_name) { + UNUSED(op); + handle_timezone(var_name.c_str()); +} + +static void handle_magic_colon_var_change(const wcstring &op, const wcstring &var_name) { + UNUSED(op); + fix_colon_delimited_var(var_name); +} + +static void handle_locale_change(const wcstring &op, const wcstring &var_name) { + UNUSED(op); + UNUSED(var_name); + init_locale(); +} + +static void handle_curses_change(const wcstring &op, const wcstring &var_name) { + UNUSED(op); + UNUSED(var_name); + init_curses(); +} + +/// Populate the dispatch table used by `react_to_variable_change()` to efficiently call the +/// appropriate function to handle a change to a variable. +static void setup_var_dispatch_table() { + for (auto var_name : locale_variables) { + var_dispatch_table.emplace(var_name, handle_locale_change); + } + + for (auto var_name : curses_variables) { + var_dispatch_table.emplace(var_name, handle_curses_change); + } + + for (auto var_name : colon_delimited_variable) { + var_dispatch_table.emplace(var_name, handle_magic_colon_var_change); + } + + var_dispatch_table.emplace(L"fish_term256", handle_fish_term_change); + var_dispatch_table.emplace(L"fish_term24bit", handle_fish_term_change); + var_dispatch_table.emplace(L"fish_escape_delay_ms", handle_escape_delay_change); + var_dispatch_table.emplace(L"LINES", handle_term_size_change); + var_dispatch_table.emplace(L"COLUMNS", handle_term_size_change); + var_dispatch_table.emplace(L"FISH_READ_BYTE_LIMIT", handle_read_limit_change); + var_dispatch_table.emplace(L"FISH_HISTORY", handle_fish_history_change); + var_dispatch_table.emplace(L"TZ", handle_tz_change); +} + void env_init(const struct config_paths_t *paths /* or NULL */) { + setup_var_dispatch_table(); + // These variables can not be altered directly by the user. const wchar_t *const ro_keys[] = { L"status", L"history", L"_", L"PWD", L"FISH_VERSION",