Merge branch 'ss/builtin-cleanup'

"git help $cmd" unnecessarily enumerated potential command names
from the filesystem, even when $cmd is known to be a built-in.

Ideas for further optimization, primarily by killing the use of
is_in_cmdlist(), were suggested in the discussion, but they can
come as follow-ups on top of this series.

* ss/builtin-cleanup:
  builtin/help.c: speed up is_git_command() by checking for builtin commands first
  builtin/help.c: call load_command_list() only when it is needed
  git.c: consistently use the term "builtin" instead of "internal command"
This commit is contained in:
Junio C Hamano
2014-01-10 10:33:47 -08:00
4 changed files with 143 additions and 125 deletions

View File

@ -14,8 +14,8 @@ Git:
. Add the external declaration for the function to `builtin.h`. . Add the external declaration for the function to `builtin.h`.
. Add the command to `commands[]` table in `handle_internal_command()`, . Add the command to the `commands[]` table defined in `git.c`.
defined in `git.c`. The entry should look like: The entry should look like:
{ "foo", cmd_foo, <options> }, { "foo", cmd_foo, <options> },
+ +

View File

@ -27,6 +27,8 @@ extern int fmt_merge_msg(struct strbuf *in, struct strbuf *out,
extern int textconv_object(const char *path, unsigned mode, const unsigned char *sha1, int sha1_valid, char **buf, unsigned long *buf_size); extern int textconv_object(const char *path, unsigned mode, const unsigned char *sha1, int sha1_valid, char **buf, unsigned long *buf_size);
extern int is_builtin(const char *s);
extern int cmd_add(int argc, const char **argv, const char *prefix); extern int cmd_add(int argc, const char **argv, const char *prefix);
extern int cmd_annotate(int argc, const char **argv, const char *prefix); extern int cmd_annotate(int argc, const char **argv, const char *prefix);
extern int cmd_apply(int argc, const char **argv, const char *prefix); extern int cmd_apply(int argc, const char **argv, const char *prefix);

View File

@ -288,6 +288,10 @@ static struct cmdnames main_cmds, other_cmds;
static int is_git_command(const char *s) static int is_git_command(const char *s)
{ {
if (is_builtin(s))
return 1;
load_command_list("git-", &main_cmds, &other_cmds);
return is_in_cmdlist(&main_cmds, s) || return is_in_cmdlist(&main_cmds, s) ||
is_in_cmdlist(&other_cmds, s); is_in_cmdlist(&other_cmds, s);
} }
@ -449,7 +453,6 @@ int cmd_help(int argc, const char **argv, const char *prefix)
int nongit; int nongit;
const char *alias; const char *alias;
enum help_format parsed_help_format; enum help_format parsed_help_format;
load_command_list("git-", &main_cmds, &other_cmds);
argc = parse_options(argc, argv, prefix, builtin_help_options, argc = parse_options(argc, argv, prefix, builtin_help_options,
builtin_help_usage, 0); builtin_help_usage, 0);
@ -458,6 +461,7 @@ int cmd_help(int argc, const char **argv, const char *prefix)
if (show_all) { if (show_all) {
git_config(git_help_config, NULL); git_config(git_help_config, NULL);
printf(_("usage: %s%s"), _(git_usage_string), "\n\n"); printf(_("usage: %s%s"), _(git_usage_string), "\n\n");
load_command_list("git-", &main_cmds, &other_cmds);
list_commands(colopts, &main_cmds, &other_cmds); list_commands(colopts, &main_cmds, &other_cmds);
} }

30
git.c
View File

@ -332,9 +332,6 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
return 0; return 0;
} }
static void handle_internal_command(int argc, const char **argv)
{
const char *cmd = argv[0];
static struct cmd_struct commands[] = { static struct cmd_struct commands[] = {
{ "add", cmd_add, RUN_SETUP | NEED_WORK_TREE }, { "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
{ "annotate", cmd_annotate, RUN_SETUP }, { "annotate", cmd_annotate, RUN_SETUP },
@ -450,6 +447,21 @@ static void handle_internal_command(int argc, const char **argv)
{ "whatchanged", cmd_whatchanged, RUN_SETUP }, { "whatchanged", cmd_whatchanged, RUN_SETUP },
{ "write-tree", cmd_write_tree, RUN_SETUP }, { "write-tree", cmd_write_tree, RUN_SETUP },
}; };
int is_builtin(const char *s)
{
int i;
for (i = 0; i < ARRAY_SIZE(commands); i++) {
struct cmd_struct *p = commands+i;
if (!strcmp(s, p->cmd))
return 1;
}
return 0;
}
static void handle_builtin(int argc, const char **argv)
{
const char *cmd = argv[0];
int i; int i;
static const char ext[] = STRIP_EXTENSION; static const char ext[] = STRIP_EXTENSION;
@ -517,8 +529,8 @@ static int run_argv(int *argcp, const char ***argv)
int done_alias = 0; int done_alias = 0;
while (1) { while (1) {
/* See if it's an internal command */ /* See if it's a builtin */
handle_internal_command(*argcp, *argv); handle_builtin(*argcp, *argv);
/* .. then try the external ones */ /* .. then try the external ones */
execv_dashed_external(*argv); execv_dashed_external(*argv);
@ -563,14 +575,14 @@ int main(int argc, char **av)
* - cannot execute it externally (since it would just do * - cannot execute it externally (since it would just do
* the same thing over again) * the same thing over again)
* *
* So we just directly call the internal command handler, and * So we just directly call the builtin handler, and die if
* die if that one cannot handle it. * that one cannot handle it.
*/ */
if (starts_with(cmd, "git-")) { if (starts_with(cmd, "git-")) {
cmd += 4; cmd += 4;
argv[0] = cmd; argv[0] = cmd;
handle_internal_command(argc, argv); handle_builtin(argc, argv);
die("cannot handle %s internally", cmd); die("cannot handle %s as a builtin", cmd);
} }
/* Look for flags.. */ /* Look for flags.. */