git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "René Scharfe" <rene.scharfe@lsrfire.ath.cx>
To: unlisted-recipients:; (no To-header on input)
Cc: Jeff King <peff@peff.net>, John Hsing <tsyj2007@gmail.com>,
	Matthieu Moy <matthieu.moy@grenoble-inp.fr>,
	git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: [PATCH] read-cache.c: fix index memory allocation
Date: Mon, 24 Oct 2011 03:01:27 +0200	[thread overview]
Message-ID: <4EA4B8E7.5070106@lsrfire.ath.cx> (raw)
In-Reply-To: <4EA453D3.7080002@lsrfire.ath.cx>

estimate_cache_size() tries to guess how much memory is needed for the
in-memory representation of an index file.  It does that by using the
file size, the number of entries and the difference of the sizes of the
on-disk and in-memory structs -- without having to check the length of
the name of each entry, which varies for each entry, but their sums are
the same no matter the representation.

Except there can be a difference.  First of all, the size is really
calculated by ce_size and ondisk_ce_size based on offsetof(..., name),
not sizeof, which can be different.  And entries are padded with 1 to 8
NULs at the end (after the variable name) to make their total length a
multiple of eight.

So in order to allocate enough memory to hold the index, change the
delta calculation to be based on offsetof(..., name) and round up to
the next multiple of eight.

On a 32-bit Linux, this delta was used before:

	sizeof(struct cache_entry)        == 72
	sizeof(struct ondisk_cache_entry) == 64
	                                    ---
	                                      8

The actual difference for an entry with a filename length of one was,
however (find the definitions are in cache.h):

	offsetof(struct cache_entry, name)        == 72
	offsetof(struct ondisk_cache_entry, name) == 62

	ce_size        == (72 + 1 + 8) & ~7 == 80
	ondisk_ce_size == (62 + 1 + 8) & ~7 == 64
	                                      ---
	                                       16

So eight bytes less had been allocated for such entries.  The new
formula yields the correct delta:

	(72 - 62 + 7) & ~7 == 16

Reported-by: John Hsing <tsyj2007@gmail.com>
Signed-off-by: Rene Scharfe <rene.scharfe@lsrfire.ath.cx>
---
 read-cache.c            |    6 ++--
 t/t7510-status-index.sh |   50 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 53 insertions(+), 3 deletions(-)
 create mode 100755 t/t7510-status-index.sh

diff --git a/read-cache.c b/read-cache.c
index 01a0e25..5790a91 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1249,9 +1249,9 @@ static void convert_from_disk(struct ondisk_cache_entry *ondisk, struct cache_en
 
 static inline size_t estimate_cache_size(size_t ondisk_size, unsigned int entries)
 {
-	long per_entry;
-
-	per_entry = sizeof(struct cache_entry) - sizeof(struct ondisk_cache_entry);
+	size_t fix_size_mem = offsetof(struct cache_entry, name);
+	size_t fix_size_dsk = offsetof(struct ondisk_cache_entry, name);
+	long per_entry = (fix_size_mem - fix_size_dsk + 7) & ~7;
 
 	/*
 	 * Alignment can cause differences. This should be "alignof", but
diff --git a/t/t7510-status-index.sh b/t/t7510-status-index.sh
new file mode 100755
index 0000000..bca359d
--- /dev/null
+++ b/t/t7510-status-index.sh
@@ -0,0 +1,50 @@
+#!/bin/sh
+
+test_description='git status with certain file name lengths'
+
+. ./test-lib.sh
+
+files="0 1 2 3 4 5 6 7 8 9 a b c d e f g h i j k l m n o p q r s t u v w x y z"
+
+check() {
+	len=$1
+	prefix=$2
+
+	for i in $files
+	do
+		: >$prefix$i
+	done
+
+	test_expect_success "status, filename length $len" "
+		git add $prefix* &&
+		git status
+	"
+	rm $prefix* .git/index
+}
+
+check  1
+check  2 p
+check  3 pr
+check  4 pre
+check  5 pref
+check  6 prefi
+check  7 prefix
+check  8 prefix-
+check  9 prefix-p
+check 10 prefix-pr
+check 11 prefix-pre
+check 12 prefix-pref
+check 13 prefix-prefi
+check 14 prefix-prefix
+check 15 prefix-prefix-
+check 16 prefix-prefix-p
+check 17 prefix-prefix-pr
+check 18 prefix-prefix-pre
+check 19 prefix-prefix-pref
+check 20 prefix-prefix-prefi
+check 21 prefix-prefix-prefix
+check 22 prefix-prefix-prefix-
+check 23 prefix-prefix-prefix-p
+check 24 prefix-prefix-prefix-pr
+
+test_done
-- 
1.7.7

  reply	other threads:[~2011-10-24  1:01 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-10-22  0:20 a bug when execute "git status" in git version 1.7.7.431.g89633 John Hsing
2011-10-23  8:25 ` Matthieu Moy
2011-10-23  8:35   ` John Hsing
2011-10-23 13:25     ` René Scharfe
2011-10-23 14:28       ` René Scharfe
2011-10-23 16:29       ` Jeff King
2011-10-23 17:50         ` René Scharfe
2011-10-24  1:01           ` René Scharfe [this message]
2011-10-24  7:07             ` [PATCH] read-cache.c: fix index memory allocation Junio C Hamano
2011-10-24 15:59               ` René Scharfe
2011-10-24 21:59               ` René Scharfe
2011-10-24 23:34                 ` Nguyen Thai Ngoc Duy
2011-10-25  0:01                   ` Nguyen Thai Ngoc Duy
2011-10-25 18:00                     ` René Scharfe
2011-10-25 16:24                 ` Junio C Hamano
2011-10-24  7:28             ` Junio C Hamano
2011-10-24 15:52               ` René Scharfe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4EA4B8E7.5070106@lsrfire.ath.cx \
    --to=rene.scharfe@lsrfire.ath.cx \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=matthieu.moy@grenoble-inp.fr \
    --cc=peff@peff.net \
    --cc=tsyj2007@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).