From: Taylor Blau <me@ttaylorr•com>
To: Patrick Steinhardt <ps@pks•im>
Cc: git@vger•kernel.org, Jeff King <peff@peff•net>
Subject: Re: [PATCH 7/8] packfile: always add packfiles to MRU when adding a pack
Date: Wed, 29 Oct 2025 19:25:03 -0400 [thread overview]
Message-ID: <aQKiT9JA+3zF4DHA@nand.local> (raw)
In-Reply-To: <20251028-pks-packfiles-store-drop-list-v1-7-1a3b82030a7a@pks.im>
On Tue, Oct 28, 2025 at 12:08:37PM +0100, Patrick Steinhardt wrote:
> When adding a packfile to it store we add it both to the list and map of
> packfiles, but we don't append it to the most-recently-used list of
> packs. We do know to add the packfile to the MRU list as soon as we
> access any of its objects, but in between we're being inconistent. It
> doesn't help that there are some subsystems that _do_ add the packfile
> to the MRU after having added it, which only adds to the confusion.
>
> Refactor the code so that we unconditionally add packfiles to the MRU
> when adding them to a packfile store.
Reading this, I thought that the MRU cache lazily added packs only upon
a successful object lookup, but looking more closely,
packfile_store_prepare_mru() adds all of the known packs to the MRU
cache eagerly.
I think I would probably advocate in the long term that we go the other
way here, which would be to avoid adding packs to the MRU cache until we
have found an object within them. But that is a larger change, since we
don't add packs outside of the MRU cache to them, only move packs which
are already in the MRU cache around.
But I think in the immediate term what you wrote here makes sense, and
it makes the behavior consistent in the meantime.
Thanks,
Taylor
next prev parent reply other threads:[~2025-10-29 23:25 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-10-28 11:08 [PATCH 0/8] packfiles: track pack lists via the packfile store Patrick Steinhardt
2025-10-28 11:08 ` [PATCH 1/8] packfile: use a `strmap` to store packs by name Patrick Steinhardt
2025-10-29 22:16 ` Taylor Blau
2025-10-28 11:08 ` [PATCH 2/8] packfile: move the MRU list into the packfile store Patrick Steinhardt
2025-10-29 22:39 ` Taylor Blau
2025-10-30 8:59 ` Patrick Steinhardt
2025-10-28 11:08 ` [PATCH 3/8] http: refactor subsystem to use `packfile_list`s Patrick Steinhardt
2025-10-29 14:24 ` Toon Claes
2025-10-30 8:58 ` Patrick Steinhardt
2025-10-28 11:08 ` [PATCH 4/8] packfile: fix approximation of object counts Patrick Steinhardt
2025-10-29 22:49 ` Taylor Blau
2025-10-30 8:58 ` Patrick Steinhardt
2025-10-28 11:08 ` [PATCH 5/8] builtin/pack-objects: simplify logic to find kept or nonlocal objects Patrick Steinhardt
2025-10-29 14:55 ` Toon Claes
2025-10-29 23:15 ` Taylor Blau
2025-10-30 8:59 ` Patrick Steinhardt
2025-10-29 23:13 ` Taylor Blau
2025-10-30 8:58 ` Patrick Steinhardt
2025-10-30 9:31 ` Toon Claes
2025-10-30 9:52 ` Patrick Steinhardt
2025-10-28 11:08 ` [PATCH 6/8] packfile: move list of packs into the packfile store Patrick Steinhardt
2025-10-28 11:08 ` [PATCH 7/8] packfile: always add packfiles to MRU when adding a pack Patrick Steinhardt
2025-10-29 23:25 ` Taylor Blau [this message]
2025-10-30 8:58 ` Patrick Steinhardt
2025-10-28 11:08 ` [PATCH 8/8] packfile: track packs via the MRU list exclusively Patrick Steinhardt
2025-10-30 10:38 ` [PATCH v2 0/8] packfiles: track pack lists via the packfile store Patrick Steinhardt
2025-10-30 10:38 ` [PATCH v2 1/8] packfile: use a `strmap` to store packs by name Patrick Steinhardt
2025-10-30 10:38 ` [PATCH v2 2/8] packfile: move the MRU list into the packfile store Patrick Steinhardt
2025-10-30 10:38 ` [PATCH v2 3/8] http: refactor subsystem to use `packfile_list`s Patrick Steinhardt
2025-10-30 10:38 ` [PATCH v2 4/8] packfile: fix approximation of object counts Patrick Steinhardt
2025-10-30 10:38 ` [PATCH v2 5/8] builtin/pack-objects: simplify logic to find kept or nonlocal objects Patrick Steinhardt
2025-10-30 10:38 ` [PATCH v2 6/8] packfile: move list of packs into the packfile store Patrick Steinhardt
2025-10-30 10:38 ` [PATCH v2 7/8] packfile: always add packfiles to MRU when adding a pack Patrick Steinhardt
2025-10-30 10:38 ` [PATCH v2 8/8] packfile: track packs via the MRU list exclusively Patrick Steinhardt
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aQKiT9JA+3zF4DHA@nand.local \
--to=me@ttaylorr$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=peff@peff$(echo .)net \
--cc=ps@pks$(echo .)im \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox