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

Re: [PATCH v2 04/10] packfile: refactor misleading code when unusing pack windows

From
Toon Claes <toon@iotcl.com>
Date
Jan 7, 2026, 10:13 UTC
Message-ID
<87fr8hpw7c.fsf@iotcl.com>
In-Reply-To
<20251218-b4-pks-pack-store-via-source-v2-4-62849007ce21@pks.im>
Patrick Steinhardt <ps@pks.im> writes:
Show 28 quoted lines
> The function `unuse_one_window()` is responsible for unmapping one of
> the packfile windows, which is done when we have exceeded the allowed
> number of window.
>
> The function receives a `struct packed_git` as input, which serves as an
> additional packfile that should be considered to be closed. If not
> given, we seemingly skip that and instead go through all of the
> repository's packfiles. The conditional that checks whether we have a
> packfile though does not make much sense anymore, as we dereference the
> packfile regardless of whether or not it is a `NULL` pointer to derive
> the repository's packfile store.
>
> The function was originally introduced via f0e17e86e1 (pack: move
> release_pack_memory(), 2017-08-18), and here we indeed had a caller that
> passed a `NULL` pointer. That caller was later removed via 9827d4c185
> (packfile: drop release_pack_memory(), 2019-08-12), so starting with
> that commit we always pass a `struct packed_git`. In 9c5ce06d74
> (packfile: use `repository` from `packed_git` directly, 2024-12-03) we
> then inadvertently started to rely on the fact that the pointer is never
> `NULL` because we use it now to identify the repository.
>
> Arguably, it didn't really make sense in the first place that the caller
> provides a packfile, as the selected window would have been overridden
> anyway by the subsequent loop over all packfiles if there was an older
> window. So the overall logic is quite misleading overall. The only case
> where it _could_ make a difference is when there were two packfiles with
> the same `last_used` value, but that case doesn't ever happen because
> the `pack_used_ctr` is strictly increasing.
I didn't even think about this edge case, so thanks for clarifying.
> Refactor the code so that we instead pass in the object database to
> help make the code less misleading.
Nice improvement.
-- 
Cheers,
Toon
Previous: Patrick SteinhardtNext: Patrick Steinhardt
Message 28 of 52 in “Start tracking packfiles per object database source”
  1. 00/10 Start tracking packfiles per object database sourcePatrick Steinhardt, Dec 15, 2025
  2. 01/10 packfile: create store via its owning sourcePatrick Steinhardt, Dec 15, 2025
  3. Justin ToblerDec 15, 2025
  4. Patrick SteinhardtDec 16, 2025
  5. 02/10 packfile: pass source to `prepare_pack()`Patrick Steinhardt, Dec 15, 2025
  6. Justin ToblerDec 15, 2025
  7. 03/10 packfile: refactor kept-pack cache to work with packfile storesPatrick Steinhardt, Dec 15, 2025
  8. Justin ToblerDec 15, 2025
  9. Patrick SteinhardtDec 16, 2025
  10. 04/10 packfile: refactor misleading code when unusing pack windowsPatrick Steinhardt, Dec 15, 2025
  11. 05/10 packfile: move packfile store into object sourcePatrick Steinhardt, Dec 15, 2025
  12. Justin ToblerDec 18, 2025
  13. Patrick SteinhardtDec 18, 2025
  14. 06/10 packfile: only prepare owning store in `packfile_store_get_packs()`Patrick Steinhardt, Dec 15, 2025
  15. Justin ToblerDec 18, 2025
  16. 07/10 packfile: only prepare owning store in `packfile_store_prepare()`Patrick Steinhardt, Dec 15, 2025
  17. 08/10 packfile: inline `find_kept_pack_entry()`Patrick Steinhardt, Dec 15, 2025
  18. Justin ToblerDec 18, 2025
  19. Patrick SteinhardtDec 18, 2025
  20. 09/10 packfile: refactor `find_pack_entry()` to work on the packfile storePatrick Steinhardt, Dec 15, 2025
  21. 10/10 packfile: move MIDX into packfile storePatrick Steinhardt, Dec 15, 2025
  22. 00/10 Start tracking packfiles per object database sourcePatrick Steinhardt, Dec 18, 2025
  23. 01/10 packfile: create store via its owning sourcePatrick Steinhardt, Dec 18, 2025
  24. 02/10 packfile: pass source to `prepare_pack()`Patrick Steinhardt, Dec 18, 2025
  25. 03/10 packfile: refactor kept-pack cache to work with packfile storesPatrick Steinhardt, Dec 18, 2025
  26. Toon ClaesJan 6, 2026
  27. 04/10 packfile: refactor misleading code when unusing pack windowsPatrick Steinhardt, Dec 18, 2025
  28. Toon ClaesJan 7, 2026
  29. 05/10 packfile: move packfile store into object sourcePatrick Steinhardt, Dec 18, 2025
  30. Toon ClaesJan 7, 2026
  31. 06/10 packfile: only prepare owning store in `packfile_store_get_packs()`Patrick Steinhardt, Dec 18, 2025
  32. 07/10 packfile: only prepare owning store in `packfile_store_prepare()`Patrick Steinhardt, Dec 18, 2025
  33. Toon ClaesJan 7, 2026
  34. 08/10 packfile: inline `find_kept_pack_entry()`Patrick Steinhardt, Dec 18, 2025
  35. Kristoffer HaugsbakkJan 8, 2026
  36. Patrick SteinhardtJan 9, 2026
  37. 09/10 packfile: refactor `find_pack_entry()` to work on the packfile storePatrick Steinhardt, Dec 18, 2025
  38. 10/10 packfile: move MIDX into packfile storePatrick Steinhardt, Dec 18, 2025
  39. 00/10 Start tracking packfiles per object database sourcePatrick Steinhardt, Jan 9, 2026
  40. 01/10 packfile: create store via its owning sourcePatrick Steinhardt, Jan 9, 2026
  41. 02/10 packfile: pass source to `prepare_pack()`Patrick Steinhardt, Jan 9, 2026
  42. 03/10 packfile: refactor kept-pack cache to work with packfile storesPatrick Steinhardt, Jan 9, 2026
  43. 04/10 packfile: refactor misleading code when unusing pack windowsPatrick Steinhardt, Jan 9, 2026
  44. Karthik NayakJan 12, 2026
  45. 05/10 packfile: move packfile store into object sourcePatrick Steinhardt, Jan 9, 2026
  46. 06/10 packfile: only prepare owning store in `packfile_store_get_packs()`Patrick Steinhardt, Jan 9, 2026
  47. 07/10 packfile: only prepare owning store in `packfile_store_prepare()`Patrick Steinhardt, Jan 9, 2026
  48. 08/10 packfile: inline `find_kept_pack_entry()`Patrick Steinhardt, Jan 9, 2026
  49. 09/10 packfile: refactor `find_pack_entry()` to work on the packfile storePatrick Steinhardt, Jan 9, 2026
  50. 10/10 packfile: move MIDX into packfile storePatrick Steinhardt, Jan 9, 2026
  51. Junio C HamanoJan 11, 2026
  52. Justin ToblerJan 12, 2026

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.