Re: [PATCH v2] commit: remove padding from struct commit

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

"Kristofer Karlsson via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> Since the order of the members no longer reflects their importance,

Micronit: "no longer reflects logical grouping or impotance" or
something, just like you already have in the comment in the header
file.

> diff --git a/commit.h b/commit.h
> index 1061ed791b..d43cb1e46d 100644
> --- a/commit.h
> +++ b/commit.h
> @@ -23,10 +23,30 @@ struct commit_list {
>   * The size of this struct matters in full repo walk operations like
>   * 'git clone' or 'git gc'. Consider using commit-slab to attach data
>   * to a commit instead of adding new fields here.
> + *
> + * The order of the members is partly chosen to keep the memory layout
> + * compact (avoiding alignment padding), and does not necessarily
> + * reflect their importance or grouping.
>   */
>  struct commit {
> +	/*
> +	 * Must be the first member, so that a "struct object *" can be
> +	 * cast to a "struct commit *".
> +	 */

Either append "... when we know it is a commit", swap the order, or
do both.  My preference is the second one which would leave the
shortest result.

>  	struct object object;
> +
> +	/*
> +	 * Index into the commit-slab arrays: unique per in-core commit,
> +	 * and only meaningful within the current process.
> +	 */
> +	unsigned int index;

Nice.  It is on a rather longer side and "and only meaningful..."
may be redundant after saying "in-core", but nothing is wrong.

Thanks.  Will queue.