Re: [PATCH 05/22] read-cache: add index reading api

[Thomas Gummerer <t.gummerer@gmail.com>]

Junio C Hamano <gitster@pobox.com> writes:

> Thomas Gummerer <t.gummerer@gmail.com> writes:
>
>>> The reader often needs to rewind the read-pointer partially while
>>> walking the index (e.g. next_cache_entry() in unpack-trees.c and how
>>> the o->cache_bottom position is used throughout the subsystem).  I
>>> am not sure if this singly-linked list is a good way to go.
>>
>> I'm not very familiar with the unpack-trees code, but from a quick look
>> the pointer (or position in the cache) is always only moved forward.
>
> I am more worried about o->cache_bottom processing, where it
> currently is an index into an array.
>
> With your ce->next_in_list_of_read_entries change, a natural rewrite
> would be to point at the ce with o->cache_bottom, but then that
> would mean you cannot in-place replace the entries like we used to
> be able to in an array based implementation.
>
> But your series does not seem to touch unpack-trees yet, so I may be
> worried too much before it becomes necessary.

Yes, you're right, as Duy mentioned in the other email I just responded
to it makes sense to keep the index around for now.

I looked at the unpack-trees code a bit, and adding a new api and hiding
index_state->cache[] will probably be a bit harder to do than I
originally thought, so it's best to keep that around for now, as we're
still able to get the benefits from partial loading even if it's not
hidden.