[PATCH] commit: remove padding from struct commit

[PATCH] commit: remove padding from struct commit

[Kristofer Karlsson via GitGitGadget]

From: Kristofer Karlsson <krka@spotify.com>

On 64-bit platforms, "struct commit" is 80 bytes, 8 of which are
padding:
 - 4 bytes between .object and .date
 - 4 bytes of tail padding after .index

Reduce memory usage by moving .index into the hole after .object.
On 64-bit platforms this shrinks the struct to 72 bytes and
eliminates the padding, resulting in 10% less memory for every commit
allocated, with no change in behaviour.

On platforms where 8-byte members only need 4-byte alignment (e.g.
i386) both layouts are already free of padding, so the size stays the
same there.

Measuring peak RSS of "git rev-list --all" over an internal monorepo
with ~4.5 million commits (three runs each):

             peak RSS     time
  before     ~849 MiB   12.8-13.7s
  after      ~817 MiB   12.8-13.0s

The ~32 MiB saved is close to the expected 8 bytes * 4.5M commits
(~34 MiB); running time is unchanged within noise.

Signed-off-by: Kristofer Karlsson <krka@spotify.com>
---
    commit: remove padding from struct commit
    
    I stumbled on this while rabbit-holing on an unrelated change to the
    commit walk, by looking at the layout with gdb's "ptype /o".
    
    Digging into the history showed that the size and padding of "struct
    commit" have changed several times, sometimes undoing earlier
    optimizations.
    
    I also prototyped a regression test that compares the size of "struct
    commit" and "struct object" against the sum of their members' sizes,
    plus any expected padding. I left it out of this patch because I am not
    sure whether:
    
     1. such a test is worth the cost of updating it whenever members are
        added or removed, or
    
     2. a unit test is the right place for it.
    
    I am happy to send it as a follow-up if there is interest.

Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-2309%2Fspkrka%2Fcommit-index-hole-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-2309/spkrka/commit-index-hole-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/2309

 commit.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/commit.h b/commit.h
index 1061ed791b..66a5d245f2 100644
--- a/commit.h
+++ b/commit.h
@@ -26,6 +26,7 @@ struct commit_list {
  */
 struct commit {
 	struct object object;
+	unsigned int index;
 	timestamp_t date;
 	struct commit_list *parents;
 
@@ -35,7 +36,6 @@ struct commit {
 	 * or get_commit_tree_oid().
 	 */
 	struct tree *maybe_tree;
-	unsigned int index;
 };
 
 extern int save_commit_buffer;

base-commit: 1891de46f79dc444a47db1c87bd0c8c76b5463f2
-- 
gitgitgadget

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

[Derrick Stolee]

On 11/12/2026 11:14 AM, Kristofer Karlsson via GitGitGadget wrote:
> From: Kristofer Karlsson <krka@spotify.com>
> 
> On 64-bit platforms, "struct commit" is 80 bytes, 8 of which are
> padding:
>  - 4 bytes between .object and .date
>  - 4 bytes of tail padding after .index

Good find.

> Reduce memory usage by moving .index into the hole after .object.
> On 64-bit platforms this shrinks the struct to 72 bytes and
> eliminates the padding, resulting in 10% less memory for every commit
> allocated, with no change in behaviour.
> 
> On platforms where 8-byte members only need 4-byte alignment (e.g.
> i386) both layouts are already free of padding, so the size stays the
> same there.
> 
> Measuring peak RSS of "git rev-list --all" over an internal monorepo
> with ~4.5 million commits (three runs each):
> 
>              peak RSS     time
>   before     ~849 MiB   12.8-13.7s
>   after      ~817 MiB   12.8-13.0s
> 
> The ~32 MiB saved is close to the expected 8 bytes * 4.5M commits
> (~34 MiB); running time is unchanged within noise.

Thanks for measuring the memory load improvement. And of course this
won't have any impact on 32-bit machines as the struct will use
4-byte alignment by default.

>  struct commit {
>  	struct object object;
> +	unsigned int index;
>  	timestamp_t date;
>  	struct commit_list *parents;
>  
> @@ -35,7 +36,6 @@ struct commit {
>  	 * or get_commit_tree_oid().
>  	 */
>  	struct tree *maybe_tree;
> -	unsigned int index;
>  };
This certainly looks like it should be obviously correct. I
believe that the only order-critical part is the object
struct needing to be first so we can "cast" a commit to an
object.

Thanks,
-Stolee

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

[Junio C Hamano]

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

> From: Kristofer Karlsson <krka@spotify.com>
>
> On 64-bit platforms, "struct commit" is 80 bytes, 8 of which are
> padding:
>  - 4 bytes between .object and .date
>  - 4 bytes of tail padding after .index
> ...
> Measuring peak RSS of "git rev-list --all" over an internal monorepo
> with ~4.5 million commits (three runs each):
>
>              peak RSS     time
>   before     ~849 MiB   12.8-13.7s
>   after      ~817 MiB   12.8-13.0s
>
> The ~32 MiB saved is close to the expected 8 bytes * 4.5M commits
> (~34 MiB); running time is unchanged within noise.
>
> Signed-off-by: Kristofer Karlsson <krka@spotify.com>
> ---
>     I also prototyped a regression test that compares the size of "struct
>     commit" and "struct object" against the sum of their members' sizes,
>     plus any expected padding. I left it out of this patch because I am not
>     sure whether:
>     
>      1. such a test is worth the cost of updating it whenever members are
>         added or removed, or
>     
>      2. a unit test is the right place for it.
>     
>     I am happy to send it as a follow-up if there is interest.

The comment before this structure says ...

> diff --git a/commit.h b/commit.h
> index 1061ed791b..66a5d245f2 100644
> --- a/commit.h
> +++ b/commit.h
> @@ -26,6 +26,7 @@ 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.
 */

but apparently it did not have much effect to prevent this from
happening.

I would think it is an effective way to force people to think struct
size bloat of important and populous structure to always measure and
warn.  There may be better ways to achieve the same that others may
think of, but I very much appreciate your bringing it up, even only
as a discussion starter.

>  struct commit {
>  	struct object object;
> +	unsigned int index;
>  	timestamp_t date;
>  	struct commit_list *parents;
>  
> @@ -35,7 +36,6 @@ struct commit {
>  	 * or get_commit_tree_oid().
>  	 */
>  	struct tree *maybe_tree;
> -	unsigned int index;
>  };

Obviously, ordering members based on size, not their logical
relationships, has its downside.  The "object" being at offset 0 is
an integral part of our in-core object design (fake inheritance in
C), but the updated layout gives a false sense that somehow "index"
may have a more important meaning than say "date" or "parents".

We may want to have some comments next to "index" (which is a unique
number given to all in-core commits while they are held in memory,
to allow the commit slab subsystem to use it as an index into flat
array of things) and "date" (which is the committer date taken from
the underlying commit object, without any adjustment), which are two
of these members whose meanings are not immediately obvious.