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

Re: [PATCH v4] index-pack: remove fetch_if_missing=0

From
Kousik Sanagavarapu <five231003@gmail.com>
Date
Mar 19, 2023, 06:17 UTC
Message-ID
<92c321c4-7968-e993-4157-f0d06edb9283@gmail.com>
In-Reply-To
<xmqqlejvf0j6.fsf@gitster.g>
On 18/03/23 04:28, Junio C Hamano wrote:
Show 38 quoted lines
> Kousik Sanagavarapu <five231003@gmail.com> writes:
>
>> A collision test is triggered in sha1_object(), whenever there is an
>> object file in our repo. If our repo is a partial clone, then checking
>> for this file existence does not lazy-fetch the object (if the object
>> is missing and if there are one or more promisor remotes) when
>> fetch_if_missing is set to 0.
>>
>> Though this global lets us control lazy-fetching in regions of code,
>> it prevents multi-threading [1].
> Sorry, but I really do not see the point.
>
> We already have read_lock/read_unlock to prevent multiple threads
> from stomping on the in-core object database structure either way.
>
> If somebody needs to dynamically change the value of fetch_if_missing
> after the program started and spawned multiple threads, yes, the update
> to the single variable would become a problem point in multi-threading.
>
> But that is not what we are doing, and you already discovered that
> this was done as "a temporary measure" to selectively let some
> programs use 0 and others use 1 for lazy-fetching, at a very early
> part of these programs.
>
> If we are to reduce this global, perhaps we should teach more
> codepaths not to lazy fetch by default.  Once everybody gets
> converted like so, then index-pack can lose the assignment of 0 to
> the variable, as the global variable would be initialized to 0 and
> nobody will flip it to 1 to "temporarily opt into lazy fetching by
> default until it gets fixed".  At that point, we can lose the global
> variable.
>
> So "we want to reduce the use of this global" is not a good reason
> to do this change at all, without a convincing argument that says
> why everybody should do automatic lazy fetching of objects.  If
> everybody should avoid doing automatic lazy fetching, a good first
> step to reduce the use of this global is not to touch index-pack
> that has already been fixed not to do so, no?
Thanks for the review.
Also, thanks for pointing out the direction of work in this area.
Really helpful.
Previous: Junio C HamanoNext: Sean Allred
Message 18 of 20 in “index-pack: remove fetch_if_missing=0”
  1. index-pack: remove fetch_if_missing=0Kousik Sanagavarapu, Feb 25, 2023
  2. Kousik SanagavarapuFeb 27, 2023
  3. Jonathan TanFeb 27, 2023
  4. Kousik SanagavarapuFeb 28, 2023
  5. index-pack: remove fetch_if_missing=0Kousik Sanagavarapu, Mar 10, 2023
  6. Junio C HamanoMar 10, 2023
  7. Jonathan TanMar 10, 2023
  8. Junio C HamanoMar 10, 2023
  9. Jonathan TanMar 11, 2023
  10. Kousik SanagavarapuMar 12, 2023
  11. Kousik SanagavarapuMar 11, 2023
  12. Kousik SanagavarapuMar 11, 2023
  13. index-pack: remove fetch_if_missing=0Kousik Sanagavarapu, Mar 13, 2023
  14. Junio C HamanoMar 13, 2023
  15. Junio C HamanoMar 13, 2023
  16. index-pack: remove fetch_if_missing=0Kousik Sanagavarapu, Mar 17, 2023
  17. Junio C HamanoMar 17, 2023
  18. Kousik SanagavarapuMar 19, 2023
  19. Sean AllredMar 11, 2023
  20. Junio C HamanoMar 11, 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.