Stefan Sperling <stsp@stsp.name> wrote:
> This patch avoids a loop over the ID set. This loops removes
> object from the set which have reused deltas. Instead, we can
> skip such items in another loop which happens further below.
> 
> My goal is simply to have less looping over the object set, even
> though this doesn't make a measurable difference in performance.
> 
> Patch applies on top of the compressed-delta reuse diff.
> 
> ok?

Agree on merging the two loops, it's also easier to read.  ok op

(it also means that we're freeing data later than before, but it
shouldn't be an issue i guess)

> diff 19b445e416490300e818e4a7b91f10bde1fb3832 fe8175d3d5b2cce6073a5a6f382e6076aeac824f
> blob - fb1e3545cc2cf36d2fb0c5ebec569c5b2302472d
> blob + ca90c2008f13c7d8d23491fdf83419879cbc986d
> --- lib/pack_create.c
> +++ lib/pack_create.c
> @@ -1870,23 +1870,14 @@ remove_unused_object(struct got_object_id *id, void *d
>  }
>  
>  static const struct got_error *
> -remove_reused_object(struct got_object_id *id, void *data, void *arg)
> -{
> -	struct got_object_idset *idset = arg;
> -	struct got_pack_meta *m = data;
> -
> -	if (m->have_reused_delta)
> -		got_object_idset_remove(NULL, idset, id);
> -
> -	return NULL;
> -}
> -
> -static const struct got_error *
>  add_meta_idset_cb(struct got_object_id *id, void *data, void *arg)
>  {
>  	struct got_pack_meta *m = data;
>  	struct got_pack_metavec *v = arg;
>  
> +	if (m->have_reused_delta)
> +		return NULL;
> +
>  	return add_meta(m, v);
>  }
>  
> @@ -1905,6 +1896,7 @@ got_pack_create(uint8_t *packsha1, FILE *packfile,
>  	struct got_ratelimit rl;
>  	struct got_pack_metavec deltify, reuse;
>  	int ncolored = 0, nfound = 0, ntrees = 0;
> +	size_t ndeltify;
>  
>  	memset(&deltify, 0, sizeof(deltify));
>  	memset(&reuse, 0, sizeof(reuse));
> @@ -1956,12 +1948,6 @@ got_pack_create(uint8_t *packsha1, FILE *packfile,
>  	    cancel_cb, cancel_arg);
>  	if (err)
>  		goto done;
> -	if (reuse.nmeta > 0) {
> -		err = got_object_idset_for_each(idset,
> -		    remove_reused_object, idset);
> -		if (err)
> -			goto done;
> -	}
>  
>  	delta_cache = fdopen(delta_cache_fd, "a+");
>  	if (delta_cache == NULL) {
> @@ -1975,23 +1961,27 @@ got_pack_create(uint8_t *packsha1, FILE *packfile,
>  		goto done;
>  	}
>  
> -	deltify.meta = calloc(got_object_idset_num_elements(idset),
> -	    sizeof(struct got_pack_meta *));
> -	if (deltify.meta == NULL) {
> -		err = got_error_from_errno("calloc");
> -		goto done;
> -	}
> -	deltify.metasz = got_object_idset_num_elements(idset);
> +	ndeltify = got_object_idset_num_elements(idset) - reuse.nmeta;
> +	if (ndeltify > 0) {
> +		deltify.meta = calloc(ndeltify, sizeof(struct got_pack_meta *));
> +		if (deltify.meta == NULL) {
> +			err = got_error_from_errno("calloc");
> +			goto done;
> +		}
> +		deltify.metasz = ndeltify;
>  
> -	err = got_object_idset_for_each(idset, add_meta_idset_cb, &deltify);
> -	if (err)
> -		goto done;
> -	if (deltify.nmeta > 0) {
> -		err = pick_deltas(deltify.meta, deltify.nmeta, ncolored,
> -		    nfound, ntrees, nours, reuse.nmeta, delta_cache, repo,
> -		    progress_cb, progress_arg, &rl, cancel_cb, cancel_arg);
> +		err = got_object_idset_for_each(idset, add_meta_idset_cb,
> +		    &deltify);
>  		if (err)
>  			goto done;
> +		if (deltify.nmeta > 0) {
> +			err = pick_deltas(deltify.meta, deltify.nmeta,
> +			    ncolored, nfound, ntrees, nours, reuse.nmeta,
> +			    delta_cache, repo, progress_cb, progress_arg, &rl,
> +			    cancel_cb, cancel_arg);
> +			if (err)
> +				goto done;
> +		}
>  	}
>  
>  	if (fflush(delta_cache) == EOF) {