Convert list of builtins from sorted array to unordered_set

The order of this list does not need to be strictly maintained any
longer.

Benchmarked with `hyperfine` as follows, where `bench1` is the existing
approach of binary search and `bench2` is the new unordered_set code,
(executed under bash because fish would always return non-zero). The
benchmark code checks each argv to see if it is a builtin keyword (both
return the same result):

```
hyperfine './bench1 $(shuf /usr/share/dict/words)' './bench2 $(shuf /usr/share/dict/words)'
Benchmark #1: ./bench1 $(shuf /usr/share/dict/words)

  Time (mean ± σ):      68.4 ms ±   3.0 ms    [User: 28.8 ms, System: 38.9 ms]

  Range (min … max):    60.4 ms …  75.4 ms

Benchmark #2: ./bench2 $(shuf /usr/share/dict/words)

  Time (mean ± σ):      61.4 ms ±   2.3 ms    [User: 23.1 ms, System: 39.8 ms]

  Range (min … max):    58.1 ms …  67.1 ms

Summary

'./bench2 $(shuf /usr/share/dict/words)' ran
    1.11x faster than './bench1 $(shuf /usr/share/dict/words)'
```
This commit is contained in:
Mahmoud Al-Qudsi 2018-04-03 14:57:02 -05:00
parent c492d03f51
commit d8a1928c24
2 changed files with 34 additions and 36 deletions

View file

@ -26,6 +26,7 @@
#include <algorithm> #include <algorithm>
#include <memory> #include <memory>
#include <string> #include <string>
#include <unordered_set>
#include "builtin.h" #include "builtin.h"
#include "builtin_argparse.h" #include "builtin_argparse.h"
@ -75,14 +76,6 @@
#include "wgetopt.h" #include "wgetopt.h"
#include "wutil.h" // IWYU pragma: keep #include "wutil.h" // IWYU pragma: keep
bool builtin_data_t::operator<(const wcstring &other) const {
return wcscmp(this->name, other.c_str()) < 0;
}
bool builtin_data_t::operator<(const builtin_data_t *other) const {
return wcscmp(this->name, other->name) < 0;
}
/// Counts the number of arguments in the specified null-terminated array /// Counts the number of arguments in the specified null-terminated array
int builtin_count_args(const wchar_t *const *argv) { int builtin_count_args(const wchar_t *const *argv) {
int argc; int argc;
@ -401,12 +394,10 @@ int builtin_false(parser_t &parser, io_streams_t &streams, wchar_t **argv) {
// END OF BUILTIN COMMANDS // END OF BUILTIN COMMANDS
// Below are functions for handling the builtin commands. // Below are functions for handling the builtin commands.
// THESE MUST BE SORTED BY NAME! Completion lookup uses binary search.
// Data about all the builtin commands in fish. // Data about all the builtin commands in fish.
// Functions that are bound to builtin_generic are handled directly by the parser. // Functions that are bound to builtin_generic are handled directly by the parser.
// NOTE: These must be kept in sorted order! static const std::unordered_set<builtin_data_t> builtin_datas = {
static const builtin_data_t builtin_datas[] = {
{L"[", &builtin_test, N_(L"Test a condition")}, {L"[", &builtin_test, N_(L"Test a condition")},
{L"and", &builtin_generic, N_(L"Execute command if previous command suceeded")}, {L"and", &builtin_generic, N_(L"Execute command if previous command suceeded")},
{L"argparse", &builtin_argparse, N_(L"Parse options in fish script")}, {L"argparse", &builtin_argparse, N_(L"Parse options in fish script")},
@ -415,8 +406,7 @@ static const builtin_data_t builtin_datas[] = {
{L"bind", &builtin_bind, N_(L"Handle fish key bindings")}, {L"bind", &builtin_bind, N_(L"Handle fish key bindings")},
{L"block", &builtin_block, N_(L"Temporarily block delivery of events")}, {L"block", &builtin_block, N_(L"Temporarily block delivery of events")},
{L"break", &builtin_break_continue, N_(L"Stop the innermost loop")}, {L"break", &builtin_break_continue, N_(L"Stop the innermost loop")},
{L"breakpoint", &builtin_breakpoint, {L"breakpoint", &builtin_breakpoint, N_(L"Temporarily halt execution of a script and launch an interactive debug prompt")},
N_(L"Temporarily halt execution of a script and launch an interactive debug prompt")},
{L"builtin", &builtin_builtin, N_(L"Run a builtin command instead of a function")}, {L"builtin", &builtin_builtin, N_(L"Run a builtin command instead of a function")},
{L"case", &builtin_generic, N_(L"Conditionally execute a block of commands")}, {L"case", &builtin_generic, N_(L"Conditionally execute a block of commands")},
{L"cd", &builtin_cd, N_(L"Change working directory")}, {L"cd", &builtin_cd, N_(L"Change working directory")},
@ -424,8 +414,7 @@ static const builtin_data_t builtin_datas[] = {
{L"commandline", &builtin_commandline, N_(L"Set or get the commandline")}, {L"commandline", &builtin_commandline, N_(L"Set or get the commandline")},
{L"complete", &builtin_complete, N_(L"Edit command specific completions")}, {L"complete", &builtin_complete, N_(L"Edit command specific completions")},
{L"contains", &builtin_contains, N_(L"Search for a specified string in a list")}, {L"contains", &builtin_contains, N_(L"Search for a specified string in a list")},
{L"continue", &builtin_break_continue, {L"continue", &builtin_break_continue, N_(L"Skip the rest of the current lap of the innermost loop")},
N_(L"Skip the rest of the current lap of the innermost loop")},
{L"count", &builtin_count, N_(L"Count the number of arguments")}, {L"count", &builtin_count, N_(L"Count the number of arguments")},
{L"disown", &builtin_disown, N_(L"Remove job from job list")}, {L"disown", &builtin_disown, N_(L"Remove job from job list")},
{L"echo", &builtin_echo, N_(L"Print arguments")}, {L"echo", &builtin_echo, N_(L"Print arguments")},
@ -463,8 +452,6 @@ static const builtin_data_t builtin_datas[] = {
{L"wait", &builtin_wait, N_(L"Wait for background processes completed")}, {L"wait", &builtin_wait, N_(L"Wait for background processes completed")},
{L"while", &builtin_generic, N_(L"Perform a command multiple times")}}; {L"while", &builtin_generic, N_(L"Perform a command multiple times")}};
#define BUILTIN_COUNT (sizeof builtin_datas / sizeof *builtin_datas)
/// Look up a builtin_data_t for a specified builtin /// Look up a builtin_data_t for a specified builtin
/// ///
/// @param name /// @param name
@ -474,18 +461,19 @@ static const builtin_data_t builtin_datas[] = {
/// Pointer to a builtin_data_t /// Pointer to a builtin_data_t
/// ///
static const builtin_data_t *builtin_lookup(const wcstring &name) { static const builtin_data_t *builtin_lookup(const wcstring &name) {
const builtin_data_t *array_end = builtin_datas + BUILTIN_COUNT; auto search = builtin_data_t { name };
const builtin_data_t *found = std::lower_bound(builtin_datas, array_end, name); auto result = builtin_datas.find(search);
if (found != array_end && name == found->name) { if (result == builtin_datas.end()) {
return found; return NULL;
} }
return NULL;
return &*result;
} }
/// Initialize builtin data. /// Initialize builtin data.
void builtin_init() { void builtin_init() {
for (size_t i = 0; i < BUILTIN_COUNT; i++) { for (auto &bi : builtin_datas) {
intern_static(builtin_datas[i].name); intern_static(bi.name.c_str());
} }
} }
@ -529,9 +517,9 @@ int builtin_run(parser_t &parser, const wchar_t *const *argv, io_streams_t &stre
/// Returns a list of all builtin names. /// Returns a list of all builtin names.
wcstring_list_t builtin_get_names() { wcstring_list_t builtin_get_names() {
wcstring_list_t result; wcstring_list_t result;
result.reserve(BUILTIN_COUNT); result.reserve(builtin_datas.size());
for (size_t i = 0; i < BUILTIN_COUNT; i++) { for (auto &bi : builtin_datas) {
result.push_back(builtin_datas[i].name); result.push_back(bi.name);
} }
return result; return result;
} }
@ -539,9 +527,9 @@ wcstring_list_t builtin_get_names() {
/// Insert all builtin names into list. /// Insert all builtin names into list.
void builtin_get_names(std::vector<completion_t> *list) { void builtin_get_names(std::vector<completion_t> *list) {
assert(list != NULL); assert(list != NULL);
list->reserve(list->size() + BUILTIN_COUNT); list->reserve(list->size() + builtin_datas.size());
for (size_t i = 0; i < BUILTIN_COUNT; i++) { for (auto &bi : builtin_datas) {
append_completion(list, builtin_datas[i].name); append_completion(list, bi.name);
} }
} }
@ -550,7 +538,7 @@ wcstring builtin_get_desc(const wcstring &name) {
wcstring result; wcstring result;
const builtin_data_t *builtin = builtin_lookup(name); const builtin_data_t *builtin = builtin_lookup(name);
if (builtin) { if (builtin) {
result = _(builtin->desc); result = _(builtin->desc.c_str());
} }
return result; return result;
} }

View file

@ -16,16 +16,26 @@ struct io_streams_t;
/// Data structure to describe a builtin. /// Data structure to describe a builtin.
struct builtin_data_t { struct builtin_data_t {
// Name of the builtin. // Name of the builtin.
const wchar_t *name; const wcstring name;
// Function pointer tothe builtin implementation. // Function pointer to the builtin implementation.
int (*func)(parser_t &parser, io_streams_t &streams, wchar_t **argv); int (*func)(parser_t &parser, io_streams_t &streams, wchar_t **argv);
// Description of what the builtin does. // Description of what the builtin does.
const wchar_t *desc; const wcstring desc;
bool operator<(const wcstring &) const; bool operator == (const builtin_data_t &other) const {
bool operator<(const builtin_data_t *) const; return name == other.name;
}
}; };
namespace std {
template<>
struct hash<builtin_data_t> {
std::size_t operator()(const builtin_data_t &data) const {
return std::hash<wcstring>()(data.name);
}
};
}
/// The default prompt for the read command. /// The default prompt for the read command.
#define DEFAULT_READ_PROMPT L"set_color green; echo -n read; set_color normal; echo -n \"> \"" #define DEFAULT_READ_PROMPT L"set_color green; echo -n read; set_color normal; echo -n \"> \""