public inbox for git@vger.kernel.org 
 help / color / mirror / Atom feed

From: Derrick Stolee <stolee@gmail•com>
To: Kristofer Karlsson via GitGitGadget <gitgitgadget@gmail•com>,
	git@vger•kernel.org
Cc: Kristofer Karlsson <krka@spotify•com>
Subject: Re: [PATCH] merge: use repo_in_merge_bases for octopus up-to-date check
Date: Mon, 18 May 2026 08:20:44 -0400	[thread overview]
Message-ID: <c5b333f1-0db6-4aec-a369-6503cb924e7f@gmail.com> (raw)
In-Reply-To: <pull.2110.git.1778566286543.gitgitgadget@gmail.com>

On 5/12/2026 2:11 AM, Kristofer Karlsson via GitGitGadget wrote:
> From: Kristofer Karlsson <krka@spotify•com>
> 
> The octopus merge path checks whether each remote head is already
> an ancestor of HEAD by computing all merge-bases via
> repo_get_merge_bases() and comparing the first result's OID to
> the remote head.  This is more expensive than necessary:
> repo_get_merge_bases() calls paint_down_to_common() with
> min_generation=0, performs the full STALE drain, and may run
> remove_redundant(), when all we need is a yes/no reachability
> answer.
> 
> Replace this with repo_in_merge_bases(), which answers the
> is-ancestor question directly.  When generation numbers are
> available, repo_in_merge_bases() uses can_all_from_reach() -- a
> DFS bounded by generation number that stops as soon as the target
> is found or ruled out, without entering paint_down_to_common() at
> all.  Without generation numbers, it still benefits from a tighter
> min_generation floor.
> 
> Signed-off-by: Kristofer Karlsson <krka@spotify•com>
> ---
>     merge: use repo_in_merge_bases for octopus up-to-date check
>     
>     While reviewing callers of repo_get_merge_bases() for a different patch,
>     I noticed the octopus up-to-date loop in builtin/merge.c computes full
>     merge-bases only to check whether each remote head is an ancestor of
>     HEAD.
>     
>     The existing code calls repo_get_merge_bases(), takes the first result,
>     frees the list, and compares the OID to the remote head. This is
>     equivalent to an is-ancestor check, which repo_in_merge_bases() answers
>     directly.
>     
>     Using repo_in_merge_bases() simplifies the code (-14/+4 lines) and
>     avoids unnecessary work: with generation numbers it uses
>     can_all_from_reach() instead of paint_down_to_common(), and without
>     generation numbers it still benefits from a tighter min_generation
>     floor. In practice this only matters for octopus merges on repos with
>     deep history, so the main value here is the simplification.

The code change looks right to me. Do you have any performance numbers
to share? Or was this motivated mostly as an opportunity for code cleanup?

Thanks,
-Stolee

next prev parent reply	other threads:[~2026-05-18 12:20 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-12  6:11 [PATCH] merge: use repo_in_merge_bases for octopus up-to-date check Kristofer Karlsson via GitGitGadget
2026-05-18 12:20 ` Derrick Stolee [this message]
2026-05-18 14:38   ` Kristofer Karlsson

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=c5b333f1-0db6-4aec-a369-6503cb924e7f@gmail.com \
    --to=stolee@gmail$(echo .)com \
    --cc=git@vger$(echo .)kernel.org \
    --cc=gitgitgadget@gmail$(echo .)com \
    --cc=krka@spotify$(echo .)com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox