Re: [PATCH 2/4] odb: eagerly initialize alternates

On 26/08/10 03:33PM, Patrick Steinhardt wrote:
> When creating the object database we initialize the main object database
> source, but we don't yet initialize its alternates. Instead, we have
> many calls to `odb_prepare_alternates()` cluttered around the code base
> whenever we are about to iterate through the sources.
> 
> This lazy loading doesn't really add much value: the moment where read

Should this say "where we read" instead?

> any object we _have_ to load the alternates anyway. So given that most
> of our commands would access the object database this optimization is
> not really buying us much in the first place. Quite on the contrary, it
> makes the code harder to understand and is a potential source of bugs in
> case any callsite forgot to prepare alternates before we iterate through
> the sources.
> 
> Historically though there was a reason why we deferred lazy-loading: it
> may happen that the repository has "core.ignoreCase" configured, and we
> use that to deduplicate the list of alternates in case we had the same
> alternate configured multiple times, but with different casing. We used
> to initialize the object database before we had fully configured the
> owning repository though, and consequently we couldn't access that
> configuration yet. This has changed in the preceding commit though where
> we started to parse "core.ignoreCase" manually.
> 
> Eagerly prepare alternates both when creating the object database and
> when flushing its caches. Drop the now-unneeded calls to prepare the
> alternates that are scattered across the code base.

This sounds like the right direction and overall much simpler. Nice.

> Signed-off-by: Patrick Steinhardt <[email protected]>
> ---
[snip]
> @@ -1091,6 +1071,8 @@ struct object_database *odb_new(struct repository *repo,
>  	o->alternate_db = secondary_sources;
>  	o->inmemory_objects = &odb_source_inmemory_new(o)->base;
>  
> +	odb_prepare_alternates(o);

Now we eagerly prepare alternates at time of ODB creation.

> +
>  	free(primary_source);
>  	return o;
>  }
> @@ -1151,10 +1133,10 @@ void odb_prepare(struct object_database *o, enum odb_prepare_flags flags)
>  	 */
>  	if (flags & ODB_PREPARE_FLUSH_CACHES) {
>  		o->loaded_alternates = 0;
> +		odb_prepare_alternates(o);
>  		o->object_count_valid = 0;
>  	}

Ok we also invoke `odb_prepare_alternates()` when we need to refresh all
alternate sources. Makes sense.

The rest of this patch just removes the now-unneeded
`odb_prepare_alternates()` call sites and looks good.

-Justin