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

Re: [PATCH 4/9] submodule.c: sort changed_submodule_names before searching it

From
Stefan Beller <sbeller@google.com>
Date
Sep 12, 2018, 19:06 UTC
Message-ID
<CAGZ79kbavjVbTqXsmtjW6=jhkq47_p3mc6=92xOp4_mfhqDtvw@mail.gmail.com>
In-Reply-To
<xmqqr2hylgv9.fsf@gitster-ct.c.googlers.com>
On Wed, Sep 12, 2018 at 11:18 AM Junio C Hamano <gitster@pobox.com> wrote:
Show 35 quoted lines
>
> Stefan Beller <sbeller@google.com> writes:
>
> > We can string_list_insert() to maintain sorted-ness of the
> > list as we find new items, or we can string_list_append() to
> > build an unsorted list and sort it at the end just once.
> >
> > To pick which one is more appropriate, we notice the fact
> > that we discover new items more or less in the already
> > sorted order.  That makes "append then sort" more
> > appropriate.
>
> Sorry, but I still do not get the math you are implying in the
> second paragraph.  Are you saying that append-then-sort is efficient
> when items being appended is already sorted?  That depends on the
> sorting algorithm used, so the logic is incomplete unless you say
> "given that we use X for sorting,...", I think.
>
> Do we really discover new items in sorted order, by the way?  In a
> single diff invocation made inside collect_changed_submodules() for
> one commit in the superproject's history, we will grab changed paths
> in the pathname order (i.e. sorted); if the superproject's tip commit
> touches the submodules at paths A and Z, we will discover these two
> paths in sorted order.
>
> But because we are walking the superproject's history to collect all
> paths that have been affected in that function, and repeatedly
> calling diff as we discover commit in the superproject's history, I
> am not sure how well the resulting set of paths would be sorted.
>
> The tip commit in superproject's history may have modified the
> submodule at path X, the parent of that commit may have touched the
> submodule at path M, and its parent may have touched the submodule
> at path A.  Don't we end up grabbing these paths in that discoverd
> order, i.e. X, M and A?
That is true.
Show 9 quoted lines
>
> I still think changing it from "insert as we find an item, keeping
> the list sorted" to "append all and then sort before we start
> looking things up from the result" makes sense, but I do not think
> the "we find things in sorted order" is either true, or it would
> affect the choice between the two.  A justification to choose the
> latter I can think of that makes sense is that we don't have to pay
> cost to keep the list sorted while building it because we do not do
> any look-up while building the list.
ok.

Thanks, Stefan

Previous: Junio C HamanoNext: Stefan Beller
Message 11 of 33 in “fetch: make sure submodule oids are fetched”
  1. 0/9 fetch: make sure submodule oids are fetchedStefan Beller, Sep 11, 2018
  2. 1/9 string-list: add string_list_{pop, last} functionsStefan Beller, Sep 11, 2018
  3. Ramsay JonesSep 12, 2018
  4. Junio C HamanoSep 12, 2018
  5. 2/9 sha1-array: provide oid_array_filterStefan Beller, Sep 11, 2018
  6. Junio C HamanoSep 12, 2018
  7. 3/9 submodule.c: fix indentationStefan Beller, Sep 11, 2018
  8. Junio C HamanoSep 12, 2018
  9. 4/9 submodule.c: sort changed_submodule_names before searching itStefan Beller, Sep 11, 2018
  10. Junio C HamanoSep 12, 2018
  11. Stefan BellerSep 12, 2018
  12. 5/9 submodule: move global changed_submodule_names into fetch submodule structStefan Beller, Sep 11, 2018
  13. 6/9 submodule.c: do not copy around submodule listStefan Beller, Sep 11, 2018
  14. Ramsay JonesSep 12, 2018
  15. 7/9 submodule: fetch in submodules git directory instead of in worktreeStefan Beller, Sep 11, 2018
  16. Junio C HamanoSep 12, 2018
  17. Stefan BellerSep 13, 2018
  18. 8/9 fetch: retry fetching submodules if sha1 were not fetchedStefan Beller, Sep 11, 2018
  19. Junio C HamanoSep 12, 2018
  20. 9/9 builtin/fetch: check for submodule updates for non branch fetchesStefan Beller, Sep 11, 2018
  21. Junio C HamanoSep 12, 2018
  22. 0/9 fetch: make sure submodule oids are fetchedStefan Beller, Sep 17, 2018
  23. 1/9 string-list: add string_list_{pop, last} functionsStefan Beller, Sep 17, 2018
  24. string-list: add string_list_{pop, last} functionsStefan Beller, Sep 21, 2018
  25. 2/9 sha1-array: provide oid_array_filterStefan Beller, Sep 17, 2018
  26. Stefan BellerSep 17, 2018
  27. 3/9 submodule.c: fix indentationStefan Beller, Sep 17, 2018
  28. 4/9 submodule.c: sort changed_submodule_names before searching itStefan Beller, Sep 17, 2018
  29. 5/9 submodule: move global changed_submodule_names into fetch submodule structStefan Beller, Sep 17, 2018
  30. 6/9 submodule.c: do not copy around submodule listStefan Beller, Sep 17, 2018
  31. 7/9 submodule: fetch in submodules git directory instead of in worktreeStefan Beller, Sep 17, 2018
  32. 8/9 fetch: retry fetching submodules if needed objects were not fetchedStefan Beller, Sep 17, 2018
  33. 9/9 builtin/fetch: check for submodule updates for non branch fetchesStefan Beller, Sep 17, 2018

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.