Re: [PATCH 7/8] packfile: always add packfiles to MRU when adding a pack

From: Taylor Blau <me@ttaylorr.com>
Date: Oct 29, 2025, 23:25 UTC
Message-ID: <aQKiT9JA+3zF4DHA@nand.local>
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:

Show 9 quoted lines

> 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