Re: [GSoC Patch 2/3] commit: convert commit->generation to a slab

[Derrick Stolee <stolee@gmail.com>]

On 6/4/2020 3:27 AM, Abhishek Kumar wrote:
> In this commit, we will use the generation slab helpers introduced in
> last commit and replace existing uses of commit->generation using
> 'contrib/coccinelle/generation.cocci'
> @@ -1048,7 +1048,7 @@ static void write_graph_chunk_data(struct hashfile *f, int hash_len,
>  		else
>  			packedDate[0] = 0;
>  
> -		packedDate[0] |= htonl((*list)->generation << 2);
> +		packedDate[0] |= htonl(generation((*list)) << 2);

nit: We no longer need the extra parens around *list.

> @@ -3414,8 +3414,8 @@ static void init_topo_walk(struct rev_info *revs)
>  		test_flag_and_insert(&info->explore_queue, c, TOPO_WALK_EXPLORED);
>  		test_flag_and_insert(&info->indegree_queue, c, TOPO_WALK_INDEGREE);
>  
> -		if (c->generation < info->min_generation)
> -			info->min_generation = c->generation;
> +		if (generation(c) < info->min_generation)
> +			info->min_generation = generation(c);

A pattern I've noticed in several places is that the struct
member is accessed multiple times in the same method body,
and this is auto-converted to multiple method calls. However,
these values are fixed, so it would be better to store the
value as a local variable and reuse that variable instead.

This is one of the shortcomings of the Coccinelle transformation,
so you'll need to manually inspect each of the diff fragments to
see if we can reduce the number of method calls. It might be
helpful to do that as a follow-up, so we can see that this patch
is generated by the Coccinelle script, and then a later patch can
be scrutinized more carefully when you are doing manual code
manipulation.

Thanks,
-Stolee