Re: [PATCH 2/2] Speed up is_git_command() by checking early for internal commands

2014-01-02 Thread Sebastian Schuberth 

On 02.01.2014 09:51, Christian Couder wrote:


diff --git a/builtin/help.c b/builtin/help.c
index b6fc15e..1f0261e 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -284,10 +284,15 @@ static int git_help_config(const char *var, const char 
*value, void *cb)
 return git_default_config(var, value, cb);
  }

+extern int is_internal_command(const char *s);
+


Starting the new year in keeping with the fine tradition of asking
people who add stuff to clean up what others left behind, I would
suggest moving all the code related to internal commands (or maybe all
commands) in a new pair of files like "internal-cmds.{c,h}". This way
git.c and builtin/help.c could include internal-cmds.h and you
wouldn't need such extern declaration.


Wouldn't the existing builtin.h be a more appropriate for this? (And 
create a builtin.c for the implementations.)


Also, I start to realize that it's a bit unfortunate that we seem to use 
the terms "builtin" and "internal command" interchangeably. I'll 
probably add a patch to address this.


--
Sebastian Schuberth
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 2/2] Speed up is_git_command() by checking early for internal commands

2014-01-02 Thread Christian Couder 
On Mon, Dec 30, 2013 at 10:07 PM, Sebastian Schuberth
 wrote:
> Since 2dce956 is_git_command() was a bit slow as it does file I/O in the
> call to list_commands_in_dir(). Avoid the file I/O by adding an early
> check for internal commands.
>
> Signed-off-by: Sebastian Schuberth 
> ---
>  builtin/help.c |   5 ++
>  git.c  | 242 
> ++---
>  2 files changed, 132 insertions(+), 115 deletions(-)
>
> diff --git a/builtin/help.c b/builtin/help.c
> index b6fc15e..1f0261e 100644
> --- a/builtin/help.c
> +++ b/builtin/help.c
> @@ -284,10 +284,15 @@ static int git_help_config(const char *var, const char 
> *value, void *cb)
> return git_default_config(var, value, cb);
>  }
>
> +extern int is_internal_command(const char *s);
> +

Starting the new year in keeping with the fine tradition of asking
people who add stuff to clean up what others left behind, I would
suggest moving all the code related to internal commands (or maybe all
commands) in a new pair of files like "internal-cmds.{c,h}". This way
git.c and builtin/help.c could include internal-cmds.h and you
wouldn't need such extern declaration.

Happy new year everyone,
Christian.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH 2/2] Speed up is_git_command() by checking early for internal commands

2013-12-30 Thread Sebastian Schuberth 
Since 2dce956 is_git_command() was a bit slow as it does file I/O in the
call to list_commands_in_dir(). Avoid the file I/O by adding an early
check for internal commands.

Signed-off-by: Sebastian Schuberth 
---
 builtin/help.c |   5 ++
 git.c  | 242 ++---
 2 files changed, 132 insertions(+), 115 deletions(-)

diff --git a/builtin/help.c b/builtin/help.c
index b6fc15e..1f0261e 100644
--- a/builtin/help.c
+++ b/builtin/help.c
@@ -284,10 +284,15 @@ static int git_help_config(const char *var, const char 
*value, void *cb)
return git_default_config(var, value, cb);
 }
 
+extern int is_internal_command(const char *s);
+
 static struct cmdnames main_cmds, other_cmds;
 
 static int is_git_command(const char *s)
 {
+   if (is_internal_command(s))
+   return 1;
+
load_command_list("git-", &main_cmds, &other_cmds);
return is_in_cmdlist(&main_cmds, s) ||
is_in_cmdlist(&other_cmds, s);
diff --git a/git.c b/git.c
index 3799514..cc81138 100644
--- a/git.c
+++ b/git.c
@@ -332,124 +332,136 @@ static int run_builtin(struct cmd_struct *p, int argc, 
const char **argv)
return 0;
 }
 
+static struct cmd_struct commands[] = {
+   { "add", cmd_add, RUN_SETUP | NEED_WORK_TREE },
+   { "annotate", cmd_annotate, RUN_SETUP },
+   { "apply", cmd_apply, RUN_SETUP_GENTLY },
+   { "archive", cmd_archive },
+   { "bisect--helper", cmd_bisect__helper, RUN_SETUP },
+   { "blame", cmd_blame, RUN_SETUP },
+   { "branch", cmd_branch, RUN_SETUP },
+   { "bundle", cmd_bundle, RUN_SETUP_GENTLY },
+   { "cat-file", cmd_cat_file, RUN_SETUP },
+   { "check-attr", cmd_check_attr, RUN_SETUP },
+   { "check-ignore", cmd_check_ignore, RUN_SETUP | NEED_WORK_TREE },
+   { "check-mailmap", cmd_check_mailmap, RUN_SETUP },
+   { "check-ref-format", cmd_check_ref_format },
+   { "checkout", cmd_checkout, RUN_SETUP | NEED_WORK_TREE },
+   { "checkout-index", cmd_checkout_index,
+   RUN_SETUP | NEED_WORK_TREE},
+   { "cherry", cmd_cherry, RUN_SETUP },
+   { "cherry-pick", cmd_cherry_pick, RUN_SETUP | NEED_WORK_TREE },
+   { "clean", cmd_clean, RUN_SETUP | NEED_WORK_TREE },
+   { "clone", cmd_clone },
+   { "column", cmd_column, RUN_SETUP_GENTLY },
+   { "commit", cmd_commit, RUN_SETUP | NEED_WORK_TREE },
+   { "commit-tree", cmd_commit_tree, RUN_SETUP },
+   { "config", cmd_config, RUN_SETUP_GENTLY },
+   { "count-objects", cmd_count_objects, RUN_SETUP },
+   { "credential", cmd_credential, RUN_SETUP_GENTLY },
+   { "describe", cmd_describe, RUN_SETUP },
+   { "diff", cmd_diff },
+   { "diff-files", cmd_diff_files, RUN_SETUP | NEED_WORK_TREE },
+   { "diff-index", cmd_diff_index, RUN_SETUP },
+   { "diff-tree", cmd_diff_tree, RUN_SETUP },
+   { "fast-export", cmd_fast_export, RUN_SETUP },
+   { "fetch", cmd_fetch, RUN_SETUP },
+   { "fetch-pack", cmd_fetch_pack, RUN_SETUP },
+   { "fmt-merge-msg", cmd_fmt_merge_msg, RUN_SETUP },
+   { "for-each-ref", cmd_for_each_ref, RUN_SETUP },
+   { "format-patch", cmd_format_patch, RUN_SETUP },
+   { "fsck", cmd_fsck, RUN_SETUP },
+   { "fsck-objects", cmd_fsck, RUN_SETUP },
+   { "gc", cmd_gc, RUN_SETUP },
+   { "get-tar-commit-id", cmd_get_tar_commit_id },
+   { "grep", cmd_grep, RUN_SETUP_GENTLY },
+   { "hash-object", cmd_hash_object },
+   { "help", cmd_help },
+   { "index-pack", cmd_index_pack, RUN_SETUP_GENTLY },
+   { "init", cmd_init_db },
+   { "init-db", cmd_init_db },
+   { "log", cmd_log, RUN_SETUP },
+   { "ls-files", cmd_ls_files, RUN_SETUP },
+   { "ls-remote", cmd_ls_remote, RUN_SETUP_GENTLY },
+   { "ls-tree", cmd_ls_tree, RUN_SETUP },
+   { "mailinfo", cmd_mailinfo },
+   { "mailsplit", cmd_mailsplit },
+   { "merge", cmd_merge, RUN_SETUP | NEED_WORK_TREE },
+   { "merge-base", cmd_merge_base, RUN_SETUP },
+   { "merge-file", cmd_merge_file, RUN_SETUP_GENTLY },
+   { "merge-index", cmd_merge_index, RUN_SETUP },
+   { "merge-ours", cmd_merge_ours, RUN_SETUP },
+   { "merge-recursive", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
+   { "merge-recursive-ours", cmd_merge_recursive, RUN_SETUP | 
NEED_WORK_TREE },
+   { "merge-recursive-theirs", cmd_merge_recursive, RUN_SETUP | 
NEED_WORK_TREE },
+   { "merge-subtree", cmd_merge_recursive, RUN_SETUP | NEED_WORK_TREE },
+   { "merge-tree", cmd_merge_tree, RUN_SETUP },
+   { "mktag", cmd_mktag, RUN_SETUP },
+   { "mktree", cmd_mktree, RUN_SETUP },
+   { "mv", cmd_mv, RUN_SETUP | NEED_WORK_TREE },
+   { "name-rev", cmd_name_rev, RUN_SETUP },
+   { "notes", cmd_notes, RUN_SETUP },
+   { "pack-objects", cmd_pack_objects, RUN_SETUP },
+   { "pack-redundant", cmd_pack_redundant, RUN_SETUP },
+   { "pack-refs", cmd_pack_refs, RUN_SETUP },
+   { "