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

Re: [PATCH 2/3] path-walk: fix setup of pending objects

From
Derrick Stolee <stolee@gmail.com>
Date
Aug 21, 2025, 20:33 UTC
Message-ID
<b37e2690-fe6a-4fde-ab7e-58368c2914c3@gmail.com>
In-Reply-To
<aKbSRQJCPh3Lsew8@pks.im>
On 8/21/2025 4:01 AM, Patrick Steinhardt wrote:
Show 18 quoted lines
> On Wed, Aug 20, 2025 at 06:39:55PM +0000, Derrick Stolee via GitGitGadget wrote:
>> From: Derrick Stolee <stolee@gmail.com>
>>
>> The previous change established a buggy instance of 'git repack -adf
>> --path-walk' when there exist paths that are tracked in the index and
>> that is the only instance of those paths in the history of the
>> repository. This change fixes that bug.
>>
>> The core problem here is that the "maybe_interesting" member of 'struct
>> type_and_oid_list' is not initialized to '1'. This member was added in
>> 6333e7ae0b (path-walk: mark trees and blobs as UNINTERESTING,
>> 2024-12-20) in a way to help when creating packfiles for a small commit
>> range using the sparse path algorithm (enabled by pack.useSparse=true).
>>
>> The idea here is that the list is marked as "maybe_interesting" if an
>> object is added that does not have the UNINITERSTING flag on it. Later,
> 
> s/UNINITERSTING/UNINTERESTING/
Thanks!
Show 15 quoted lines
>> this is checked again in case all objects in the list were marked
>> UNINTERESTING after that point in time. In this case, the algorithm
>> skips the list as there is no reason to visit it.
>>
>> This leads to the problem where the "maybe_interesting" member was not
>> appropriately initialized when the list is created from pending objects.
>> This is the fix for now.
>>
>> To help avoid this from happening in the future, a follow-up change will
>> make initializing lists use a shared method instead of allowing for an
>> update to this initialization process to miss some existing copies.
> 
> Yeah, I wanted to say that this feels quite fragile to me and very easy
> to miss. Does this mechanism buy us a lot of performance in the first
> place? Because if not we might as well just remove it entirely.

The details for the space savings and moderate time cost are listed in e5394794a5 (pack-objects: thread the path-based compression, 2025-05-16). These improvements are on top of those from --name-hash-version=2 by using a different way to focus delta calculations.

A larger, internal example for this can be seen in this table (based on testing today):

Mode | Size | Time ----------+----------+------ version 1 | 16.0 GB | 83min version 2 | 9.9 GB | 77min path walk | 6.4 GB | 74min

> But if the answer is "yes" then adding APIs around it feels like a good
> alternative.

Making the code less brittle to changes is good, but also I'm interested in ways to improve our test infrastructure or adding defensive features. I mentioned a couple of ideas in an earlier message.

Thanks, -Stolee

Previous: Patrick SteinhardtNext: Elijah Newren
Message 13 of 22 in “[2.51.0 Bug] Missing singleton objects in 'git repack -adf --path-walk'”
  1. 0/3 [2.51.0 Bug] Missing singleton objects in 'git repack -adf --path-walk'Derrick Stolee via GitGitGadget, Aug 20, 2025
  2. 1/3 t7700: add failing --path-walk testDerrick Stolee via GitGitGadget, Aug 20, 2025
  3. Patrick SteinhardtAug 21, 2025
  4. Derrick StoleeAug 21, 2025
  5. Junio C HamanoAug 21, 2025
  6. Elijah NewrenAug 21, 2025
  7. 2/3 path-walk: fix setup of pending objectsDerrick Stolee via GitGitGadget, Aug 20, 2025
  8. Junio C HamanoAug 20, 2025
  9. Derrick StoleeAug 20, 2025
  10. Patrick SteinhardtAug 21, 2025
  11. Derrick StoleeAug 21, 2025
  12. Patrick SteinhardtAug 21, 2025
  13. Derrick StoleeAug 21, 2025
  14. Elijah NewrenAug 21, 2025
  15. 3/3 path-walk: create initializer for path listsDerrick Stolee via GitGitGadget, Aug 20, 2025
  16. Elijah NewrenAug 21, 2025
  17. 0/2 [2.51.0 Bug] Missing singleton objects in 'git repack -adf --path-walk'Derrick Stolee via GitGitGadget, Aug 25, 2025
  18. 1/2 path-walk: fix setup of pending objectsDerrick Stolee via GitGitGadget, Aug 25, 2025
  19. 2/2 path-walk: create initializer for path listsDerrick Stolee via GitGitGadget, Aug 25, 2025
  20. Elijah NewrenAug 26, 2025
  21. Junio C HamanoAug 26, 2025
  22. Patrick SteinhardtSep 2, 2025

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.