From: Christian Couder Date: Tue, 08 Sep 2026 17:11:48 GMT Subject: Re: [PATCH v2 1/5] promisor-remote: factor out lazy_fetch_objects() Message-ID: In-Reply-To: On Fri, Aug 14, 2026 at 7:49 PM Junio C Hamano wrote: > Perhaps writing it this way would make it easier to tell what is > going on. We try the preferred ones first, and then fall back to > the other ones. > > return (try_promisor_remotes(..., true) || > try_promisor_remotes(..., false)); Yes, this is used in v3. > But more importantly, I wonder if keeping the list of missing object > names in memory will later turn out to be problematic in real-life > applications. Without knowing much about how the current code for > bulk dehydrating promisor objects is structured, I expected an API > that looks more like: > > - bulk_download_begin(): performs the early part of > fetch_objects(), sets up connections to the promisor remote(s), > and calls start_command() on the child process. > > - bulk_download_this(): after calling the _begin() function above, > it runs around and collects missing objects that it needs to do > its work. For each such missing object it discovers, this > function is called, which sends the object name down the > '--stdin' file descriptor. > > - bulk_download_done(): tells the child process that we are done > feeding object names. > > but that is not what I am seeing. I guess the current arrangement > cannot be avoided, because we are going to fetch from more than one > promisor remote. Under such constraints, the way to deal with a > massive number of missing objects will not be "streaming" like I > imagined above, but needs to be done differently, like spooling to a > file or something silly like that. > > In any case, except that this avoids checking the environment > variable multiple times, I can see that it is a no-op refactoring of > the existing code. Yeah, I prefer to avoid working on a big refactoring in this area until we have evidence showing that there is a bottleneck here. > Nice and cleanly done. Thanks.