Re: [PATCH 11/20] refs: record the ref_store in ref_cache, not ref_dir

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Mar 20, 2017, 22:39 UTC
Message-ID: <5e015cd9-f53e-b27b-ff73-d95706df3223@alum.mit.edu>
In-Reply-To: <20170320175145.zwfqp7varl4hwink@sigill.intra.peff.net>

On 03/20/2017 06:51 PM, Jeff King wrote:

Show 13 quoted lines

> On Mon, Mar 20, 2017 at 05:33:16PM +0100, Michael Haggerty wrote:
> 
>> Instead of keeping a pointer to the ref_store in every ref_dir entry,
>> store it once in `struct ref_cache`, and change `struct ref_dir` to
>> include a pointer to its containing `ref_cache` instead. This makes it
>> easier to add to the information that is stored in `struct ref_cache`
>> without inflating the size of the individual entries.
> 
> This last sentence confused me. It's a pointer either way, no?
> 
> Do you just mean that we are free to add whatever we like to the
> "ref_cache" without polluting the "ref_store" that is a more public data
> structure?

Yeah, that's explained poorly. It might be clearer as

> This makes it easier to add to the information that is accessible
> from a `ref_dir` without increasing the size of every `ref_dir`
> instance.

It used to be that `ref_dir` contained a pointer to the `ref_store` that contains it. (BTW, such a pointer *can't* be turned into a pointer to the `ref_cache` because (1) the `ref_dir` shouldn't have to know what kind of `ref_store` it is being used with, and (2) a `packed_ref_cache` can be detached from its old `ref_cache` if the stat info indicates that the `packed-refs` file has been modified since it was last read.)

But we want to add a `fill_ref_dir` callback pointer in a place that is reachable from the `ref_dir` so that it can fill itself when necessary. We could add it to the `ref_dir` struct, but that would inflate its size. Instead, it makes more sense for the `ref_dir` to have a pointer to the enclosing `ref_cache`, and store the `fill_ref_dir` pointer in `ref_cache`. The `ref_cache` also gets a pointer to the enclosing `ref_store`; that way a `ref_dir` also has a way to access the `ref_store` that contains it.

An alternative would be to pass a pointer to the `ref_cache` down the call stack when it is being accessed, but that seemed like a lot of work that wouldn't lead to a very clean result.

Michael