Re: [PATCH v2 2/7] branch: add branch_checked_out() helper
- From
Derrick Stolee <derrickstolee@github.com>
- Date
- Jun 8, 2022, 02:14 UTC
- Message-ID
- <e84b8f19-4fb4-77cd-7e56-087e84e59817@github.com>
- In-Reply-To
- <xmqqr140t9am.fsf@gitster.g>
On 6/7/2022 6:09 PM, Junio C Hamano wrote:
Show 20 quoted lines
> "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
>> +int branch_checked_out(const char *refname, char **path)
>> +{
>> + struct worktree **worktrees = get_worktrees();
>> + const struct worktree *wt = find_shared_symref(worktrees, "HEAD", refname);
>> + int result = wt && !wt->is_bare;
>> +
>> + if (result && path)
>> + *path = xstrdup(wt->path);
>> +
>> + free_worktrees(worktrees);
>> + return result;
>> +}
>
> Don't you plan to call this repeatedly from the for_each_deco
> iteration? I am wondering if it should take the result of
> get_worktrees() and reuse the result of get_worktrees(), instead of
> enumerating the list of worktrees and freeing for each of the
> branches you need to inspect.Sorry, you had mentioned this in v1 and I just forgot to fix this.
Show 25 quoted lines
> There also was another topic > > https://lore.kernel.org/git/pull.1266.v2.git.git.1652690501963.gitgitgadget@gmail.com/ > > that was triggered by find_shared_symref() being relatively > heavy-weight, which suggests a more involved refactoring. > > I wonder if we rather want to rewrite find_shared_symref() *not* to > take the target parameter at all, and instead introduce a new > function that takes a worktree, and report the branch that is > checked out (or being operated on via rebase or bisect). Then we > can > > - create a strset out of its result, i.e. set of branches that > should not be touched; > > - iterate over refs that point into the history being rebased > (using for_each_decoration()), and consult that strset to see if > any of them is being rewritten. > > With the API of find_shared_symref(), we'd need to iterate over all > worktrees for each decoration. With such a restructuring, we can > iterate over all worktrees just once, and match the result with > decoration, so the problem becomes O(N)+O(M) and not O(N*M) for > number of worktrees N and number of decorations M.
Yes, that's a good plan. I'll take a look.
Thanks, -Stolee