From: Jonathan Tan <jonathantanmy@google.com>
Date: Tue, 24 Apr 2018 18:55:36 GMT
Subject: Re: [PATCH 9/9] cache.h: allow sha1_object_info to handle arbitrary repositories
Message-ID: <20180424115536.54fe1a62541624b2dbe06619@google.com>
In-Reply-To: <20180424184233.GA90854@google.com>

On Tue, 24 Apr 2018 11:42:33 -0700
Brandon Williams <bmwill@google.com> wrote:

> On 04/24, Jonathan Tan wrote:
> > On Mon, 23 Apr 2018 16:43:27 -0700
> > Stefan Beller <sbeller@google.com> wrote:
> > 
> > > This involves also adapting sha1_object_info_extended and a some
> > > internal functions that are used to implement these. It all has to
> > > happen in one patch, because of a single recursive chain of calls visits
> > > all these functions.
> > 
> > In packfile.c, unpack_entry() invokes get_delta_base_cache_entry(),
> > which references a global (delta_base_cache). Does delta_base_cache need
> > to be moved to the repo object (or object store object) first, or is
> > this safe?
> 
> After looking at this, I think it should be safe for now since its a
> cache that requires a packed_git pointer to access (and those would be
> per repository).  We may want to move it in to the repository at some
> point though.

Thanks for the pointer. I see that a packed_git pointer is part of the
key to that hashmap, so this is indeed safe. (Probably worth discussing
this in the commit message.)

There is another global, delta_base_cached, but it just limits the total
size of the hashmap, so it's safe to use that too.