From 5d6415b6bf50b28834f6657df30e1a3db027dae4 Mon Sep 17 00:00:00 2001 From: Kurtis Rader Date: Fri, 11 Nov 2016 15:26:16 -0800 Subject: [PATCH] use binary search for enum map lookups Switch from a linear to a binary search when looking for a matching string in an enum map. Testing shows this is a little more than twice as fast when searching for keywords in the sixteen entry keyword_map array. This speedup doesn't matter much when searching for subbcommands but any slow down in the parser is unacceptable. --- src/builtin.cpp | 22 +++++++++++++--------- src/common.h | 32 ++++++++++++++++++++++---------- 2 files changed, 35 insertions(+), 19 deletions(-) diff --git a/src/builtin.cpp b/src/builtin.cpp index 7abacf0aa..43b1b049d 100644 --- a/src/builtin.cpp +++ b/src/builtin.cpp @@ -2172,19 +2172,21 @@ enum status_cmd_t { STATUS_PRINT_STACK_TRACE, STATUS_UNDEF }; +// Must be sorted by string, not enum or random. const enum_map status_enum_map[] = { - {STATUS_IS_LOGIN, L"is-login"}, - {STATUS_IS_INTERACTIVE, L"is-interactive"}, + {STATUS_CURRENT_FILENAME, L"current-filename"}, + {STATUS_CURRENT_LINE_NUMBER, L"current-line-number"}, {STATUS_IS_BLOCK, L"is-block"}, {STATUS_IS_COMMAND_SUB, L"is-command-sub"}, {STATUS_IS_FULL_JOB_CTRL, L"is-full-job-control"}, + {STATUS_IS_INTERACTIVE, L"is-interactive"}, {STATUS_IS_INTERACTIVE_JOB_CTRL, L"is-interactive-job-control"}, + {STATUS_IS_LOGIN, L"is-login"}, {STATUS_IS_NO_JOB_CTRL, L"is-no-job-control"}, - {STATUS_CURRENT_FILENAME, L"current-filename"}, - {STATUS_CURRENT_LINE_NUMBER, L"current-line-number"}, {STATUS_SET_JOB_CONTROL, L"job-control"}, {STATUS_PRINT_STACK_TRACE, L"print-stack-trace"}, {STATUS_UNDEF, NULL}}; +#define status_enum_map_len (sizeof status_enum_map / sizeof *status_enum_map) /// Remember the status subcommand and disallow selecting more than one status subcommand. static bool set_status_cmd(wchar_t *const cmd, status_cmd_t *status_cmd, status_cmd_t sub_cmd, @@ -2348,7 +2350,7 @@ static int builtin_status(parser_t &parser, io_streams_t &streams, wchar_t **arg // If a status command hasn't already been specified via a flag check the first word. // Note that this can be simplified after we eliminate allowing subcommands as flags. if (w.woptind < argc) { - status_cmd_t subcmd = str_to_enum(argv[w.woptind], status_enum_map); + status_cmd_t subcmd = str_to_enum(argv[w.woptind], status_enum_map, status_enum_map_len); if (subcmd != STATUS_UNDEF) { if (!set_status_cmd(cmd, &status_cmd, subcmd, streams)) { return STATUS_BUILTIN_ERROR; @@ -2947,9 +2949,11 @@ static int builtin_return(parser_t &parser, io_streams_t &streams, wchar_t **arg } enum hist_cmd_t { HIST_SEARCH = 1, HIST_DELETE, HIST_CLEAR, HIST_MERGE, HIST_SAVE, HIST_UNDEF }; -const enum_map hist_enum_map[] = {{HIST_SEARCH, L"search"}, {HIST_DELETE, L"delete"}, - {HIST_CLEAR, L"clear"}, {HIST_MERGE, L"merge"}, - {HIST_SAVE, L"save"}, {HIST_UNDEF, NULL}}; +// Must be sorted by string, not enum or random. +const enum_map hist_enum_map[] = {{HIST_CLEAR, L"clear"}, {HIST_DELETE, L"delete"}, + {HIST_MERGE, L"merge"}, {HIST_SAVE, L"save"}, + {HIST_SEARCH, L"search"}, {HIST_UNDEF, NULL}}; +#define hist_enum_map_len (sizeof hist_enum_map / sizeof *hist_enum_map) /// Remember the history subcommand and disallow selecting more than one history subcommand. static bool set_hist_cmd(wchar_t *const cmd, hist_cmd_t *hist_cmd, hist_cmd_t sub_cmd, @@ -3129,7 +3133,7 @@ static int builtin_history(parser_t &parser, io_streams_t &streams, wchar_t **ar // Note that this can be simplified after we eliminate allowing subcommands as flags. // See the TODO above regarding the `long_options` array. if (w.woptind < argc) { - hist_cmd_t subcmd = str_to_enum(argv[w.woptind], hist_enum_map); + hist_cmd_t subcmd = str_to_enum(argv[w.woptind], hist_enum_map, hist_enum_map_len); if (subcmd != HIST_UNDEF) { if (!set_hist_cmd(cmd, &hist_cmd, subcmd, streams)) { return STATUS_BUILTIN_ERROR; diff --git a/src/common.h b/src/common.h index 04fb6bbd3..731c8c23e 100644 --- a/src/common.h +++ b/src/common.h @@ -781,25 +781,37 @@ long convert_digit(wchar_t d, int base); // Return true if the character is in a range reserved for fish's private use. bool fish_reserved_codepoint(wchar_t c); -/// Used for constructing mappings between enums and strings. +/// Used for constructing mappings between enums and strings. The resulting array must be sorted +/// according to the `str` member since str_to_enum() does a binary search. Also the last entry must +/// have NULL for the `str` member and the default value for `val` to be returned if the string +/// isn't found. template struct enum_map { T val; const wchar_t *const str; }; -/// Given a string return the matching enum. Return the sentinal enum if no match is made. +/// Given a string return the matching enum. Return the sentinal enum if no match is made. The map +/// must be sorted by the `str` member. A binary search is twice as fast as a linear search with 16 +/// elements in the map. template -static T str_to_enum(const wchar_t *enum_str, const enum_map map[]) { - const enum_map *entry = map; - while (entry->str) { - if (wcscmp(enum_str, entry->str) == 0) { - return entry->val; +static T str_to_enum(const wchar_t *name, const enum_map map[], int len) { + // Ignore the sentinel value when searching as it is the "not found" value. + size_t left = 0, right = len - 1; + + while (left < right) { + size_t mid = left + (right - left) / 2; + int cmp = wcscmp(name, map[mid].str); + if (cmp < 0) { + right = mid; // name was smaller than mid + } else if (cmp > 0) { + left = mid + 1; // name was larger than mid + } else { + return map[mid].val; // found it } - entry++; } - return entry->val; // return val of sentinel entry in the array -}; + return map[len - 1].val; // return the sentinel value +} /// Given an enum return the matching string. template