Re: [PATCH 4/6 (v2)] administrative functions for rev-cache, and start of integration into git

[Junio C Hamano <gitster@pobox.com>]

"Nick Edelen" <sirnot@gmail.com> writes:

Note that I didn't read the entire patch, so the parts I did not comment
on in this message may or may not have problematic parts.

>  - extra 'size' field added to commit, tag and blob objects, initialized in 
>    parse_* functions and cache traversal

It is unclear which "size" this field refers to without comment.

I do not know if this change is justifiable.  We (mostly Linus) spent
considerble effort to shrink the memory footprint of struct commit (and
struct object in general) exactly because revision traversal needs to
inspect tons of them.

> +static unsigned int parse_size(const char *name)
> +{
> +	unsigned int size;
> +	char *p;
> +	
> +	size = strtol(name, &p, 10);
> +	switch (*p) {
> +	case 'k' : 
> +	case 'K' : 
> +		size *= 0x400;
> +		break;
> +	case 'm' : 
> +	case 'M' : 
> +		size *= 0x100000;
> +		break;
> +	default : 
> +		break;
> +	}
> +	
> +	return size;
> +}

Looks vaguely familiar.

The configuration parser already knows about these size suffixes when told
to read "int".  Can't that code, e.g. git_parse_ulong(), be reused here?

> @@ -219,7 +221,6 @@ unsigned char *get_cache_slice(struct commit *commit)
>  		return 0;
>  	
>  	ie = search_index(commit->object.sha1);
> -	
>  	if (ie && ie->cache_index < idx_head.cache_nr)
>  		return idx_caches + ie->cache_index * 20;

"Oops, fix an earlier mistake"...

> @@ -262,7 +282,6 @@ static int setup_traversal(struct cache_slice_header *head, unsigned char *map,
>  	struct commit_list *prev, *wp, **wpp;
>  	int retval;
>  	
> -	/* printf("adding %s\n", sha1_to_hex(commit->object.sha1)); */

Likewise.