On Sun, Feb 16, 2025 at 8:37 AM Phillip Wood via GitGitGadget
<gitgitgadget@xxxxxxxxx> wrote:
>
> From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
>
> If a process tries to read the output from "git merge-tree --stdin"
> before it closes merge-tree's stdin then it deadlocks. This happens
> because merge-tree does not flush its output before trying to read
> another line of input and means that it is not possible to cherry-pick a
> sequence of commits using "git merge-tree --stdin". Fix this by calling
> maybe_flush_or_die() before trying to read the next line of
> input.
Makes sense.
> Flushing the output after each merge does not seem to affect the
> performance, any difference is lost in the noise even after increasing
> the number of runs.
>
> $ git rev-list --merges --parents -n100 origin/master |
> sed 's/^[^ ]* //' >/tmp/merges
> $ hyperfine -L flush 0,1 --warmup 1 --runs 30 \
> 'GIT_FLUSH={flush} ./git merge-tree --stdin </tmp/merges'
> Benchmark 1: GIT_FLUSH=0 ./git merge-tree --stdin </tmp/merges
> Time (mean ± σ): 546.6 ms ± 11.7 ms [User: 503.2 ms, System: 40.9 ms]
> Range (min … max): 535.9 ms … 567.7 ms 30 runs
>
> Benchmark 2: GIT_FLUSH=1 ./git merge-tree --stdin </tmp/merges
> Time (mean ± σ): 546.9 ms ± 12.0 ms [User: 505.9 ms, System: 38.9 ms]
> Range (min … max): 529.8 ms … 570.0 ms 30 runs
>
> Summary
> 'GIT_FLUSH=0 ./git merge-tree --stdin </tmp/merges' ran
> 1.00 ± 0.03 times faster than 'GIT_FLUSH=1 ./git merge-tree --stdin </tmp/merges'
Nice; thanks for checking and providing these stats.
> Signed-off-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
> ---
> builtin/merge-tree.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/builtin/merge-tree.c b/builtin/merge-tree.c
> index 9a6c8b4e4cf..57f4340faba 100644
> --- a/builtin/merge-tree.c
> +++ b/builtin/merge-tree.c
> @@ -18,6 +18,7 @@
> #include "tree.h"
> #include "config.h"
> #include "strvec.h"
> +#include "write-or-die.h"
>
> static int line_termination = '\n';
>
> @@ -623,6 +624,7 @@ int cmd_merge_tree(int argc,
> } else {
> die(_("malformed input line: '%s'."), buf.buf);
> }
> + maybe_flush_or_die(stdout, "stdout");
>
> if (result < 0)
> die(_("merging cannot continue; got unclean result of %d"), result);
> --
> gitgitgadget
Looks good to me.
