Re: [PATCH 2/3] added --batch option to mktree

[Josh Micich <josh.micich@gmail.com>]

On Wed, May 13, 2009 at 11:18 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> This option enables creation of many tree objects with a single process
>
>... which is desirable in what way?  how does this justify the cost of
>maintenance?  what is it used for?

I have been writing a few of my own utilities on top of basic git
commands and got mild performance gains from using the existing
'--batch' option with "git cat-file" instead of executing new "git
cat-file -p <sha>" processes for each object I wish to read.  Later I
was writing a utility for mirroring another SCM (creates many trees),
and I faced a similar performance issue with "git mktree".  After
adding the '--batch' option, I experienced about a 30% speed increase.

If you find this logic persuasive, I can fix all the other issues and
re-submit this patch.

>> +--batch::
>I think you want an blank line before this.
right


>> @@ -125,20 +125,42 @@ int cmd_mktree(int ac, const char **av, const char
>> *prefix)
>
>Linewrapped and would not apply.
sorry - gmane web interface did that.  BTW - does git have a coding
standard for maximum line length?


>What do you mean by "interactively"?  Does anybody type from the standard
>input?
I guess I was trying to stress the point that the created tree object
ids are output immediately as the complete text for each tree is
received.  Other (bad) applications I have seen wait for EOF on stdin
before sending anything to stdout, and this is exactly what I had
avoided in the impl.  Perhaps you could suggest better phrasing.


>Decl-after-statement.
>No C++ comment please;
>Lose excess {} pair;
>Style: have an explicit ";" for an empty statement.
>used = 0; /* .... */
sorry - bad habits. gotta work out how to get my compiler to warn me about these


>> +             if (is_batch_mode && got_eof && used < 1) {
>> +                     // allow input to finish with a new-line (or not)
>
>Style: have an explicit ";" for an empty statement.
>
>But more importantly, what does this comment mean?  Why do you want to be
>loose in input format validation?

I agree with your implied suggestion that tight input validation is
better.  I was actually trying to keep consistent with the way I
believe mktree works today.  The final \n is optional as far as I can
tell.