From: Stefan Beller <stefanbeller@googlemail.com>
To: Jonathan Nieder <jrnieder@gmail.com>
Cc: git@vger.kernel.org, Eric Sunshine <sunshine@sunshineco.com>
Subject: Re: [PATCH] builtins: search builtin commands via binary search.
Date: Sat, 27 Jul 2013 10:49:27 +0200 [thread overview]
Message-ID: <51F38997.9010507@googlemail.com> (raw)
In-Reply-To: <20130726205737.GI14690@google.com>
[-- Attachment #1: Type: text/plain, Size: 2762 bytes --]
On 07/26/2013 10:57 PM, Jonathan Nieder wrote:
> Hi,
>
> Stefan Beller wrote:
>
>> --- a/git.c
>> +++ b/git.c
>> @@ -309,9 +309,18 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
>> return 0;
>> }
>>
>> +static int compare_internal_command(const void *a, const void *b) {
>> + /* The first parameter is of type char* describing the name,
>> + the second is a struct cmd_struct */
>
> Style:
>
> /*
> * Multi-line comments in git look like this, with an initial
> * "/*" line, a leading "*" on each line with text, and a line
> * with '*' '/' at the end.
> */
>
> [...]
Thanks for noting, however as Eric points out, that comment was not enlightening, so I removed it.
>> @@ -447,12 +456,12 @@ static void handle_internal_command(int argc, const char **argv)
>> argv[0] = cmd = "help";
>> }
>>
>> - for (i = 0; i < ARRAY_SIZE(commands); i++) {
>> - struct cmd_struct *p = commands+i;
>> - if (strcmp(p->cmd, cmd))
>> - continue;
>> + struct cmd_struct *p = (struct cmd_struct *)bsearch(cmd, commands,
>> + ARRAY_SIZE(commands), sizeof(struct cmd_struct),
>> + compare_internal_command);
>
> No need to cast --- this is C.
Also removed.
>
> Fun. Does this result in a measurable speedup, or is it just for more
> pleasant reading?
>
> Thanks and hope that helps,
> Jonathan
>
premature optimization is the root of all evil....
I tried hard to come up with a benchmark, but this is lost in the
noise. I could not figure out a way to reproducably make sure this
patch is really faster.
So I tried to `time git add COPYING` to show it's not getting slower
for the first entries in the list as well as `git fast-external-command`
whereas the fast-external-command is just an int main() {return 0; }
to check if the external commands, which are executed after searching
through all the internals come up faster.
However I could not find a speedup.
So if the patch is accepted, it would only be for readability.
I was fiddling around with make now to include the suggestion of Eric to
check the arguments for being sorted in make. However I do not
seem to fully understand the syntax yet.
My approach would have been:
sorted_internal_cmds: git.c
{ awk '/cmd_struct commands/,/};/ { if (match($2,/"/)) print $2 }' <git.c >builtin.actual && \
sort <builtin.actual >builtin.expect && \
cmp -s builtin.expect builtin.actual && \
rm builtin.expect builtin.actual \
}
all:: sorted_internal_cmds
But then there is
$ make
...
}
/bin/sh: 5: Syntax error: end of file unexpected (expecting "}")
So I suspect the { within the shell code inside the awk parameter is messing up?
Thanks,
Stefan
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 899 bytes --]
next prev parent reply other threads:[~2013-07-27 8:49 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-07-26 20:50 [PATCH] builtins: search builtin commands via binary search Stefan Beller
2013-07-26 20:57 ` Jonathan Nieder
2013-07-27 8:49 ` Stefan Beller [this message]
2013-07-27 8:50 ` Stefan Beller
2013-07-27 9:13 ` Andreas Schwab
2013-07-28 9:05 ` Eric Sunshine
2013-07-29 16:18 ` Junio C Hamano
2013-07-27 0:26 ` Eric Sunshine
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: http://vger.kernel.org/majordomo-info.html
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=51F38997.9010507@googlemail.com \
--to=stefanbeller@googlemail.com \
--cc=git@vger.kernel.org \
--cc=jrnieder@gmail.com \
--cc=sunshine@sunshineco.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://80x24.org/mirrors/git.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).