[PATCH v2] commit: remove padding from struct commit

["Kristofer Karlsson via GitGitGadget" <gitgitgadget@gmail.com>]

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.

Since the order of the members no longer reflects their importance,
document that only "object" has to come first, and describe "index"
and "date", whose meanings are not obvious from their names.

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".
    
    Changes since v1:
    
     * Document that "object" must be the first member, and that the order
       of the other members is partly chosen for a compact memory layout
       rather than reflecting their importance (Junio).
    
     * Add comments describing "index" and "date" (Junio).
    
    A regression check for the padding (unit test or a scoped -Wpadded) and
    a build-time assertion that "object" comes first are discussed in the v1
    thread; I will send those separately.

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

Range-diff vs v1:

 1:  20de8db00d ! 1:  f8f0dbcf46 commit: remove padding from struct commit
     @@ Commit message
          i386) both layouts are already free of padding, so the size stays the
          same there.
      
     +    Since the order of the members no longer reflects their importance,
     +    document that only "object" has to come first, and describe "index"
     +    and "date", whose meanings are not obvious from their names.
     +
          Measuring peak RSS of "git rev-list --all" over an internal monorepo
          with ~4.5 million commits (three runs each):
      
     @@ Commit message
      
       ## commit.h ##
      @@ commit.h: 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 *".
     ++	 */
       	struct object object;
     ++
     ++	/*
     ++	 * Index into the commit-slab arrays: unique per in-core commit,
     ++	 * and only meaningful within the current process.
     ++	 */
      +	unsigned int index;
     ++
     ++	/*
     ++	 * The committer date taken from the commit object, without any
     ++	 * adjustment.
     ++	 */
       	timestamp_t date;
     ++
       	struct commit_list *parents;
       
     + 	/*
      @@ commit.h: struct commit {
       	 * or get_commit_tree_oid().
       	 */


 commit.h | 21 ++++++++++++++++++++-
 1 file changed, 20 insertions(+), 1 deletion(-)

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 *".
+	 */
 	struct object object;
+
+	/*
+	 * Index into the commit-slab arrays: unique per in-core commit,
+	 * and only meaningful within the current process.
+	 */
+	unsigned int index;
+
+	/*
+	 * The committer date taken from the commit object, without any
+	 * adjustment.
+	 */
 	timestamp_t date;
+
 	struct commit_list *parents;
 
 	/*
@@ -35,7 +55,6 @@ struct commit {
 	 * or get_commit_tree_oid().
 	 */
 	struct tree *maybe_tree;
-	unsigned int index;
 };
 
 extern int save_commit_buffer;

base-commit: 1891de46f79dc444a47db1c87bd0c8c76b5463f2
-- 
gitgitgadget