Re: [PATCH v5 11/11] pack-objects.h: reorder members to shrink struct object_entry

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Mar 17, 2018, 19:53 UTC
Message-ID: <87muz6cwcu.fsf@evledraar.gmail.com>
In-Reply-To: <20180317141033.21545-12-pclouds@gmail.com>

On Sat, Mar 17 2018, Nguyễn Thái Ngọc Duy jotted:

Show 5 quoted lines

> Previous patches leave lots of holes and padding in this struct. This
> patch reorders the members and shrinks the struct down to 80 bytes
> (from 136 bytes, before any field shrinking is done) with 16 bits to
> spare (and a couple more in in_pack_header_size when we really run out
> of bits).

Given what I mentioned in 87po42cwql.fsf@evledraar.gmail.com just now I think we should add this to the commit message.

    This is the last in a series of memory reduction patches (see
    "pack-objects: a bit of document about struct object_entry" for the
    first one).

    Overall they've reduced repack memory size on linux.git from 3.747G
    to 3.424G, or by around 320M, a decrease of 8.5%. The runtime of
    repack has stayed the same throughout this series. Ævar's testing on
    a big monorepo he has access to (bigger than linux.git) has shown a
    7.9% reduction, so the overall expected improvement should be
    somewhere around 8%.

    See 87po42cwql.fsf@evledraar.gmail.com on-list
    (https://public-inbox.org/git/87po42cwql.fsf@evledraar.gmail.com/)
    for more detailed numbers and a test script used to produce the
    numbers cited above.

Thanks again for working on this.

Show 54 quoted lines

> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  pack-objects.h | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/pack-objects.h b/pack-objects.h
> index 2507b157d5..8979289f5f 100644
> --- a/pack-objects.h
> +++ b/pack-objects.h
> @@ -71,35 +71,36 @@ enum dfs_state {
>   */
>  struct object_entry {
>  	struct pack_idx_entry idx;
> -	/* object uncompressed size _if_ size_valid is true */
> -	uint32_t size_;
> -	unsigned size_valid:1;
> -	unsigned in_pack_idx:OE_IN_PACK_BITS;	/* already in pack */
> +	void *delta_data;	/* cached delta (uncompressed) */
>  	off_t in_pack_offset;
> +	uint32_t hash;			/* name hint hash */
> +	uint32_t size_;	/* object uncompressed size _if_ size_valid is true */
>  	uint32_t delta_idx;	/* delta base object */
>  	uint32_t delta_child_idx; /* deltified objects who bases me */
>  	uint32_t delta_sibling_idx; /* other deltified objects who
>  				     * uses the same base as me
>  				     */
> -	void *delta_data;	/* cached delta (uncompressed) */
>  	uint32_t delta_size_:OE_DELTA_SIZE_BITS; /* delta data size (uncompressed) */
>  	uint32_t delta_size_valid:1;
> +	unsigned in_pack_idx:OE_IN_PACK_BITS;	/* already in pack */
> +	unsigned size_valid:1;
>  	unsigned z_delta_size:OE_Z_DELTA_BITS;
> +	unsigned type_valid:1;
>  	unsigned type_:TYPE_BITS;
>  	unsigned in_pack_type:TYPE_BITS; /* could be delta */
> -	unsigned type_valid:1;
> -	uint32_t hash;			/* name hint hash */
> -	unsigned char in_pack_header_size;
>  	unsigned preferred_base:1; /*
>  				    * we do not pack this, but is available
>  				    * to be used as the base object to delta
>  				    * objects against.
>  				    */
>  	unsigned no_try_delta:1;
> +	unsigned char in_pack_header_size;
>  	unsigned tagged:1; /* near the very tip of refs */
>  	unsigned filled:1; /* assigned write-order */
>  	unsigned dfs_state:OE_DFS_STATE_BITS;
>  	unsigned depth:OE_DEPTH_BITS;
> +
> +	/* size: 80, bit_padding: 16 bits */
>  };
>
>  struct packing_data {