From b837f5d68da391f3a54817bb04f1e7ec04f9cdac Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 16 Jan 2014 20:19:46 -0500 Subject: [PATCH 1/5] diff_filespec: reorder dirty_submodule macro definitions diff_filespec has a 2-bit "dirty_submodule" field and defines two flags as macros. Originally these were right next to each other, but a new field was accidentally added in between in commit 4682d85. This patch puts the field and its flags back together. Using an enum like: enum { DIRTY_SUBMODULE_UNTRACKED = 1, DIRTY_SUBMODULE_MODIFIED = 2 } dirty_submodule; would be more obvious, but it bloats the structure. Limiting the enum size like: } dirty_submodule : 2; might work, but it is not portable. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- diffcore.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diffcore.h b/diffcore.h index 1c16c8595b..f822f9e2b0 100644 --- a/diffcore.h +++ b/diffcore.h @@ -43,9 +43,9 @@ struct diff_filespec { unsigned should_free : 1; /* data should be free()'ed */ unsigned should_munmap : 1; /* data should be munmap()'ed */ unsigned dirty_submodule : 2; /* For submodules: its work tree is dirty */ - unsigned is_stdin : 1; #define DIRTY_SUBMODULE_UNTRACKED 1 #define DIRTY_SUBMODULE_MODIFIED 2 + unsigned is_stdin : 1; unsigned has_more_entries : 1; /* only appear in combined diff */ struct userdiff_driver *driver; /* data should be considered "binary"; -1 means "don't know yet" */ From 5b711b207fe78de59ebcba70e16886bffaffc73c Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 16 Jan 2014 20:20:11 -0500 Subject: [PATCH 2/5] diff_filespec: drop funcname_pattern_ident field This struct field was obsoleted by be58e70 (diff: unify external diff and funcname parsing code, 2008-10-05), but we forgot to remove it. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- diffcore.h | 1 - 1 file changed, 1 deletion(-) diff --git a/diffcore.h b/diffcore.h index f822f9e2b0..92e4d484ea 100644 --- a/diffcore.h +++ b/diffcore.h @@ -29,7 +29,6 @@ struct diff_filespec { char *path; void *data; void *cnt_data; - const char *funcname_pattern_ident; unsigned long size; int count; /* Reference count */ int xfrm_flags; /* for use by the xfrm */ From 428d52a5a56f81d3a42871b51480cecafec58fdd Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 16 Jan 2014 20:21:59 -0500 Subject: [PATCH 3/5] diff_filespec: drop xfrm_flags field The only mention of this field in the code is by some debugging code which prints it out (and it will always be zero, since we never touch it otherwise). It was obsoleted very early on by 25d5ea4 ([PATCH] Redo rename/copy detection logic., 2005-05-24). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- diff.c | 4 ++-- diffcore.h | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/diff.c b/diff.c index e34bf97120..774965307c 100644 --- a/diff.c +++ b/diff.c @@ -4120,9 +4120,9 @@ void diff_debug_filespec(struct diff_filespec *s, int x, const char *one) DIFF_FILE_VALID(s) ? "valid" : "invalid", s->mode, s->sha1_valid ? sha1_to_hex(s->sha1) : ""); - fprintf(stderr, "queue[%d] %s size %lu flags %d\n", + fprintf(stderr, "queue[%d] %s size %lu\n", x, one ? one : "", - s->size, s->xfrm_flags); + s->size); } void diff_debug_filepair(const struct diff_filepair *p, int i) diff --git a/diffcore.h b/diffcore.h index 92e4d484ea..22993e1eff 100644 --- a/diffcore.h +++ b/diffcore.h @@ -31,7 +31,6 @@ struct diff_filespec { void *cnt_data; unsigned long size; int count; /* Reference count */ - int xfrm_flags; /* for use by the xfrm */ int rename_used; /* Count of rename users */ unsigned short mode; /* file mode */ unsigned sha1_valid : 1; /* if true, use sha1 and trust mode; From b38f70a82b9628d80f5a9bc938a8861ee4dcf49e Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 16 Jan 2014 20:22:56 -0500 Subject: [PATCH 4/5] diff_filespec: reorder is_binary field The middle of the diff_filespec struct contains a mixture of ints, shorts, and bit-fields, followed by a pointer. On an x86-64 system with an LP64 or LLP64 data model (i.e., most of them), the integers and flags end up being padded out by 41 bits to put the pointer at an 8-byte boundary. After the pointer, we have the "int is_binary" field, which is only 32 bits. We end up wasting another 32 bits to pad the struct size up to a multiple of 64 bits. We can move the is_binary field before the pointer, which lets the compiler store it where we used to have padding. This shrinks the top padding to only 9 bits (from the bit-fields), and eliminates the bottom padding entirely, dropping the struct size from 88 to 80 bytes. On a 32-bit system, there is no benefit, but nor should there be any harm (we only need 4-byte alignment there, so we were already using only 9 bits of padding). Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- diffcore.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diffcore.h b/diffcore.h index 22993e1eff..d911bf0a42 100644 --- a/diffcore.h +++ b/diffcore.h @@ -45,9 +45,9 @@ struct diff_filespec { #define DIRTY_SUBMODULE_MODIFIED 2 unsigned is_stdin : 1; unsigned has_more_entries : 1; /* only appear in combined diff */ - struct userdiff_driver *driver; /* data should be considered "binary"; -1 means "don't know yet" */ int is_binary; + struct userdiff_driver *driver; }; extern struct diff_filespec *alloc_filespec(const char *); From cbfe47b67fc1072998c73e6d43cf6ad061a436f5 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Thu, 16 Jan 2014 20:25:40 -0500 Subject: [PATCH 5/5] diff_filespec: use only 2 bits for is_binary flag The is_binary flag needs only three values: -1, 0, and 1. However, we use a whole 32-bit int for it on most systems (both 32- and 64- bit). Instead, we can mark it to use only 2 bits. On 32-bit systems, this lets it end up as part of the bitfield above (saving 4 bytes). On 64-bit systems, we don't see any change (because the savings end up as padding), but it does leave room for another "free" 32-bit value to be added later. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano --- diffcore.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/diffcore.h b/diffcore.h index d911bf0a42..79de8cf28d 100644 --- a/diffcore.h +++ b/diffcore.h @@ -46,7 +46,7 @@ struct diff_filespec { unsigned is_stdin : 1; unsigned has_more_entries : 1; /* only appear in combined diff */ /* data should be considered "binary"; -1 means "don't know yet" */ - int is_binary; + int is_binary : 2; struct userdiff_driver *driver; };