git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] pack-objects.h: remove outdated pahole results

From
Taylor Blau <me@ttaylorr.com>
Date
Jun 28, 2022, 18:30 UTC
Message-ID
<1379af2e9d271b501ef3942398e7f159a9c77973.1656440978.git.me@ttaylorr.com>

The size and padding of `struct object_entry` is an important factor in determining the memory usage of `pack-objects`. For this reason, 3b13a5f263 (pack-objects: reorder members to shrink struct object_entry, 2018-04-14) added a comment containing some information from pahole indicating the size and padding of that struct.

Unfortunately, this comment hasn't been updated since 9ac3f0e5b3 (pack-objects: fix performance issues on packing large deltas, 2018-07-22), despite the size of this struct changing many times since that commit.

To see just how often the size of object_entry changes, I skimmed the first-parent history with this script:

    for sha in $(git rev-list --first-parent --reverse 9ac3f0e..)
    do
      echo -n "$sha "
      git checkout -q $sha
      make -s pack-objects.o 2>/dev/null
      pahole -C object_entry pack-objects.o | sed -n \
        -e 's/\/\* size: \([0-9]*\).*/size \1/p' \
        -e 's/\/\*.*padding: \([0-9]*\).*/padding \1/p' | xargs
    done | uniq -f1

In between each merge, the size of object_entry changes too often to record every instance here. But the important merges (along with their corresponding sizes and bit paddings) in chronological order are:

    ad635e82d6 (Merge branch 'nd/pack-objects-pack-struct', 2018-05-23) size 80 padding 4
    29d9e3e2c4 (Merge branch 'nd/pack-deltify-regression-fix', 2018-08-22) size 80 padding 9
    3ebdef2e1b (Merge branch 'jk/pack-delta-reuse-with-bitmap', 2018-09-17) size 80 padding 8
    33e4ae9c50 (Merge branch 'bc/sha-256', 2019-01-29) size 96 padding 8

(indicating that the current size of the struct is 96 bytes, with 8 padding bits).

Even though this comment was written in a good spirit, it is updated infrequently enough that is serves to confuse rather than to encourage contributors to update the appropriate values when the modify the definition of object_entry.

For that reason, eliminate the confusion by removing the comment altogether.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
Noticed this while reverting an old topic out of GitHub's fork, and
realized that this comment was severely out-of-date.
 pack-objects.h | 10 ----------
 1 file changed, 10 deletions(-)
diff --git a/pack-objects.h b/pack-objects.h
index 393b9db546..579476687c 100644
--- a/pack-objects.h
+++ b/pack-objects.h
@@ -116,16 +116,6 @@ struct object_entry {
 	unsigned dfs_state:OE_DFS_STATE_BITS;
 	unsigned depth:OE_DEPTH_BITS;
 	unsigned ext_base:1; /* delta_idx points outside packlist */
-
-	/*
-	 * pahole results on 64-bit linux (gcc and clang)
-	 *
-	 *   size: 80, bit_padding: 9 bits
-	 *
-	 * and on 32-bit (gcc)
-	 *
-	 *   size: 76, bit_padding: 9 bits
-	 */
 };

 struct packing_data {
--
2.37.0.1.g1379af2e9d
Next: Derrick Stolee
Message 1 of 7 in “pack-objects.h: remove outdated pahole results”
  1. pack-objects.h: remove outdated pahole resultsTaylor Blau, Jun 28, 2022
  2. Derrick StoleeJun 28, 2022
  3. brian m. carlsonJun 28, 2022
  4. Taylor BlauJun 28, 2022
  5. Jeff KingJul 1, 2022
  6. Taylor BlauJul 1, 2022
  7. Jeff KingJul 1, 2022

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.