Re: [PATCH v5 8/9] sequencer: try to commit without forking 'git commit'

From: Phillip Wood <phillip.wood@talktalk.net>
Date: Jan 11, 2018, 10:41 UTC
Message-ID: <5efbad65-6afe-e135-4681-ba380cd6797f@talktalk.net>
In-Reply-To: <nycvar.QRO.7.76.6.1801102332350.31@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz>

On 10/01/18 22:40, Johannes Schindelin wrote:

Show 18 quoted lines

> Hi,
> 
> On Wed, 10 Jan 2018, Jonathan Nieder wrote:
> 
>> Phillip Wood wrote:
>>
>>> From: Phillip Wood <phillip.wood@dunelm.org.uk>
>>>
>>> If the commit message does not need to be edited then create the
>>> commit without forking 'git commit'. Taking the best time of ten runs
>>> with a warm cache this reduces the time taken to cherry-pick 10
>>> commits by 27% (from 282ms to 204ms), and the time taken by 'git
>>> rebase --continue' to pick 10 commits by 45% (from 386ms to 212ms) on
>>> my computer running linux. Some of greater saving for rebase is
>>> because it no longer wastes time creating the commit summary just to
>>> throw it away.
>>
>> Neat!  Dmitry Torokhov (cc-ed) noticed[1]

Thanks for reporting and bisecting this Dmitry. When I was preparing this series I checked to see if it needed to run the 'pre-commit' hook but missed the 'prepare-commit-msg' hook.

Show 16 quoted lines

> that this causes the
>> prepare-commit-msg hook not to be invoked, which I think is
>> unintentional.  Should we check for such a hook and take the slowpath
>> when it is present?
> 
> We could also easily recreate the functionality:
> 
> 	if (find_hook("pre-commit")) {
> 		struct argv_array hook_env = ARGV_ARRAY_INIT;
> 
> 		argv_array_pushf(&hook_env, "GIT_INDEX_FILE=%s",
> 			get_index_file());
> 		argv_array_push(&hook_env, "GIT_EDITOR=:");
> 		ret = run_hook_le(hook_env.argv, "pre-commit", NULL);
> 		argv_array_clear(&hook_env);
> 	}

Thanks Johannes, though it needs to run the 'prepare-commit-msg' hook, the current code in master only runs the 'pre-commit' hook when we edit the message. I'll send a patch with a test.

Best Wishes

Phillip

Show 7 quoted lines

> (This assumes that the in-process try_to_commit() is only called if the
> commit message is not to be edited interactively, which is currently the
> case.)
> 
> Ciao,
> Dscho
>