On 03/20/2017 06:51 PM, Jeff King wrote: > 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