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

Re: [PATCHv3 00/11] Expose the submodule parallelism to the user

From
Stefan Beller <sbeller@google.com>
Date
Nov 4, 2015, 18:08 UTC
Message-ID
<CAGZ79kZhgVThVrR-gOZj3zaicG43JNgv1FTxk2S5mr__7YB5Yg@mail.gmail.com>
In-Reply-To
<xmqqk2pxbqft.fsf@gitster.mtv.corp.google.com>
On Wed, Nov 4, 2015 at 9:54 AM, Junio C Hamano <gitster@pobox.com> wrote:
Show 27 quoted lines
> Stefan Beller <sbeller@google.com> writes:
>
>> Where does it apply?
>> ---
>> This series applies on top of d075d2604c0f92045caa8d5bb6ab86cf4921a4ae (Merge
>> branch 'rs/daemon-plug-child-leak' into sb/submodule-parallel-update) and replaces
>> the previous patches in sb/submodule-parallel-update
>>
>> What does it do?
>> ---
>> This series should finish the on going efforts of parallelizing
>> submodule network traffic. The patches contain tests for clone,
>> fetch and submodule update to use the actual parallelism both via
>> command line as well as a configured option. I decided to go with
>> "submodule.jobs" for all three for now.
>
> The order of patches and where the series builds makes me suspect
> that I have been expecting too much from the "parallel-fetch" topic.
>
> I've been hoping that it would be useful for the project as a whole
> to polish the other topic and make it available to wider audience
> sooner by itself (both from "end users get improved Git early"
> aspect and from "the core machinery to be reused in follow-up
> improvements are made closer to perfection sooner" perspective).  So
> I've been expecting that "Let's fix it on Windows" change directly
> on top of sb/submodule-parallel-fetch to make that topic usable
> before everything else.

I can resend the patches on top of sb/submodule-parallel-fetch (though looking at sb/submodule-parallel-fetch..d075d2604c0f920 [Merge branch 'rs/daemon-plug-child-leak' into sb/submodule-parallel-update] I don't expect conflicts, so it would be a verbatim resend)

> Other patches in this series may require
> the child_process_cleanup() change, so they may be applied on top of
> the merge between sb/submodule-parallel-fetch (updated for Windows)
> and rs/daemon-plug-child-leak topic.

I assumed the rs/daemon-plug-child-leak topic is no feature, but cleanup. Which is why I would have expected a sb/submodule-parallel-fetch-for-windows pointing at maybe the third patch of the series on top of rs/daemon-plug-child-leak

>
> That does not seem to be what's happening here (note: I am not
> complaining; I am just trying to make sure expectation matches
> reality).  Am I reading you correctly?

I really wanted to send out just one series, my bad. The ordering made sense to me (first the run-command related fixes and then the new features in later patches)

>
> I think sb/submodule-parallel-fetch + sb/submodule-parallel-update
> as a single topic would need more time to mature to be in a tagged
> release than we have in the remainder of this cycle.
I agree on that.
Show 8 quoted lines
>  It is likely
> that the former topic has a chance to get rebased after 2.7 happens.
> And that would allow us to (1) use the child_process_cleanup() from
> get-go instead of _deinit and to (2) get the machinery right both
> for UNIX and Windows from get-go.  Which would make the result
> easier to understand.  As this is one of the more important areas,
> it matters to keep the resulting code and the rationale behind it
> understandable by reading "log --reverse -p".

So you are saying that reading the Windows cleanup patch before the s/deinit/clear/ Patch by Rene makes it way easier to understand? Which is why you would prefer another history. (Merging an updated sb/submodule-parallel-fetch again to rs/daemon-plug-child-leak or even sb/submodule-parallel-update)

Thanks, Stefan

Previous: Junio C HamanoNext: Junio C Hamano
Message 33 of 34 in “[PATCHv3 00/11] Expose the submodule parallelism to the user”
  1. Stefan BellerNov 4, 2015
  2. 01/11 run_processes_parallel: delimit intermixed task outputStefan Beller, Nov 4, 2015
  3. 02/11 run-command: report failure for degraded output just onceStefan Beller, Nov 4, 2015
  4. Junio C HamanoNov 4, 2015
  5. Stefan BellerNov 4, 2015
  6. Johannes SixtNov 4, 2015
  7. Junio C HamanoNov 4, 2015
  8. Jeff KingNov 4, 2015
  9. Junio C HamanoNov 5, 2015
  10. Jeff KingNov 5, 2015
  11. Junio C HamanoNov 5, 2015
  12. Stefan BellerNov 5, 2015
  13. Junio C HamanoNov 4, 2015
  14. Stefan BellerNov 4, 2015
  15. Junio C HamanoNov 4, 2015
  16. Stefan BellerNov 4, 2015
  17. 03/11 run-command: omit setting file descriptors to non blocking in WindowsStefan Beller, Nov 4, 2015
  18. 04/11 submodule-config: keep update strategy aroundStefan Beller, Nov 4, 2015
  19. 05/11 submodule-config: drop check against NULLStefan Beller, Nov 4, 2015
  20. 06/11 submodule-config: remove name_and_item_from_varStefan Beller, Nov 4, 2015
  21. 07/11 submodule-config: introduce parse_generic_submodule_configStefan Beller, Nov 4, 2015
  22. 08/11 fetching submodules: respect `submodule.jobs` config optionStefan Beller, Nov 4, 2015
  23. Jens LehmannNov 10, 2015
  24. Stefan BellerNov 10, 2015
  25. Jens LehmannNov 11, 2015
  26. Stefan BellerNov 11, 2015
  27. Jens LehmannNov 13, 2015
  28. Stefan BellerNov 13, 2015
  29. 09/11 git submodule update: have a dedicated helper for cloningStefan Beller, Nov 4, 2015
  30. 10/11 submodule update: expose parallelism to the userStefan Beller, Nov 4, 2015
  31. 11/11 clone: allow an explicit argument for parallel submodule clonesStefan Beller, Nov 4, 2015
  32. Junio C HamanoNov 4, 2015
  33. Stefan BellerNov 4, 2015
  34. Junio C HamanoNov 4, 2015

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.