git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v3 1/3] branch: avoid unnecessary worktrees traversals

From
Rubén Justo <rjusto@gmail.com>
Date
Feb 15, 2023, 22:00 UTC
Message-ID
<b9464970-1ba5-e2f3-58c6-145ca45b1095@gmail.com>
In-Reply-To
<20230211041644.1848341-1-jonathantanmy@google.com>
On 10-feb-2023 20:16:44, Jonathan Tan wrote:
Show 11 quoted lines
> > If we could know in advance if the renamed branch is not HEAD in any
> > worktree we could avoid calling "replace_each_worktree_head_symref()",
> > and so avoid that unnecessary second traversing.
> 
> When I first read this paragraph, I thought that the traversing involved
> was just a loop through an in-memory data structure, which is not that
> costly. It turns out that a travesal involves not only constructing
> said data structure but also reading from disk to get the necessary
> information, which indeed is very costly. I would include that in the
> commit message, but won't insist on that (perhaps it's clear to others
> what is meant by traversal).

Sorry, I should have included details about why it's costly. I'll include some in the message.

Show 15 quoted lines
> 
> > Let's rename "reject_rebase_or_bisect_branch()" to a more meaningful
> > name "die_if_branch_is_being_rebased_or_bisected()" and make it return,
> > if it does not die(), if the specified branch is HEAD in any worktree.
> > Use this new information to avoid unnecessary calls to
> > "replace_each_worktree_head_symref()".
> 
> In later patches, I see that the return value can also indicate that a
> branch is an orphan, and that for the sake of code clarity, the calling
> function had to have a variable assignment of the form oldref_is_orphan
> = (oldref_is_head > 1). If this is so, it is probably better to have
> this function return something with names. So something like
> 
>   #define IS_HEAD 4
>   #define IS_ORPHAN 8
OK.  I'll use names.
Show 15 quoted lines
>   int get_branch_usage_in_worktrees(...) {...}
> 
> and then the caller can use these constants whenever it needs to know
> what kind of branch this is.
> 
> I also see in patch 2 that we're changing what the user sees under
> certain inputs. That can be avoided if we move the dying to the caller,
> and have this function merely return when the branch is being rebased
> or bisected.
> 
>   #define IS_BISECTED 1
>   #define IS_REBASED 2
> 
> or something like that. I would prefer if user-visible behavior didn't
> change unnecessarily, and this does not seem like a necessary case.
OK.
> 
> Other than that, everything looks good.
Thanks for your review and suggestions!
Previous: Jonathan TanNext: Rubén Justo
Message 23 of 49 in “branch: operations on orphan branches”
  1. 0/2 branch: operations on orphan branchesRubén Justo, Dec 30, 2022
  2. 1/2 branch: description for orphan branch errorsRubén Justo, Dec 30, 2022
  3. Junio C HamanoJan 1, 2023
  4. Rubén JustoJan 3, 2023
  5. Junio C HamanoJan 4, 2023
  6. Rubén JustoJan 6, 2023
  7. Junio C HamanoJan 6, 2023
  8. Rubén JustoJan 7, 2023
  9. Junio C HamanoJan 7, 2023
  10. 2/2 branch: rename orphan branches in any worktreeRubén Justo, Dec 30, 2022
  11. 0/3 branch: operations on orphan branchesRubén Justo, Jan 15, 2023
  12. 1/3 avoid unnecessary worktrees traversingRubén Justo, Jan 16, 2023
  13. Junio C HamanoJan 19, 2023
  14. Rubén JustoJan 19, 2023
  15. 2/3 branch: description for orphan branch errorsRubén Justo, Jan 16, 2023
  16. 3/3 branch: rename orphan branches in any worktreeRubén Justo, Jan 16, 2023
  17. Junio C HamanoJan 19, 2023
  18. Rubén JustoJan 19, 2023
  19. 3/3 branch: rename orphan branches in any worktreeRubén Justo, Jan 16, 2023
  20. 0/3 branch: operations on orphan branchesRubén Justo, Feb 6, 2023
  21. 1/3 branch: avoid unnecessary worktrees traversalsRubén Justo, Feb 6, 2023
  22. Jonathan TanFeb 11, 2023
  23. Rubén JustoFeb 15, 2023
  24. 2/3 branch: description for orphan branch errorsRubén Justo, Feb 6, 2023
  25. 3/3 branch: rename orphan branches in any worktreeRubén Justo, Feb 6, 2023
  26. Junio C HamanoFeb 7, 2023
  27. Ævar Arnfjörð BjarmasonFeb 7, 2023
  28. Rubén JustoFeb 8, 2023
  29. Junio C HamanoFeb 8, 2023
  30. 0/3 branch: operations on orphan branchesRubén Justo, Feb 22, 2023
  31. 1/3 branch: avoid unnecessary worktrees traversalsRubén Justo, Feb 22, 2023
  32. Rubén JustoFeb 25, 2023
  33. Jonathan TanFeb 27, 2023
  34. Rubén JustoFeb 28, 2023
  35. 2/3 branch: description for orphan branch errorsRubén Justo, Feb 22, 2023
  36. Jonathan TanFeb 27, 2023
  37. Junio C HamanoFeb 27, 2023
  38. Rubén JustoFeb 28, 2023
  39. 3/3 branch: rename orphan branches in any worktreeRubén Justo, Feb 22, 2023
  40. Jonathan TanFeb 27, 2023
  41. Rubén JustoFeb 28, 2023
  42. 0/5 branch: operations on orphan branchesRubén Justo, Mar 26, 2023
  43. 1/5 branch: test for failures while renaming branchesRubén Justo, Mar 26, 2023
  44. 2/5 branch: use get_worktrees() in copy_or_rename_branch()Rubén Justo, Mar 26, 2023
  45. 3/5 branch: description for orphan branch errorsRubén Justo, Mar 26, 2023
  46. 4/5 branch: rename orphan branches in any worktreeRubén Justo, Mar 26, 2023
  47. 5/5 branch: avoid unnecessary worktrees traversalsRubén Justo, Mar 26, 2023
  48. Junio C HamanoMar 27, 2023
  49. Junio C HamanoMay 1, 2023

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.