All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Torsten Bögershausen" <tboegi@web.de>
To: Stefan Beller <sbeller@google.com>,
	ronniesahlberg@gmail.com, mhagger@alum.mit.edu,
	jrnieder@gmail.com, gitster@pobox.com
Cc: git@vger.kernel.org
Subject: Re: [PATCH 12/13] refs.c: use a bit for ref_update have_old
Date: Thu, 04 Dec 2014 17:10:05 +0100	[thread overview]
Message-ID: <5480875D.6050803@web.de> (raw)
In-Reply-To: <1417681763-32334-13-git-send-email-sbeller@google.com>

On 12/04/2014 09:29 AM, Stefan Beller wrote:
Somewhat short commit message, especially the motivation is missing.

What do we gain with this patch ?
In the struct the compiler needs to layout 2*20 bytes for the sha's.
It follows an int, typically 4 bytes.
It follows another int, now with one bit.
Then we have the pointer "struct ref_lock *lock",
which needs to be aligned on 4 byte boundary for a 32 bit processor,
or an 8 byte boundary for a 64 bit machine.

Our "1 bit int" is padded with 31 bits.
We do not gain anything in memory consumption. (unless we declare int flags
to be 31 bits, and the compiler may join "have_old" and "flags" together
into one int in memory.

But there is a price to pay:
The generated code to fiddle out the bits from an int becomes more complicated.
You need to fetch the memory from one int, mask and shift.
Some processors can do this, out of my mind some ARM can, some can not.

We need to run the compiler to look at the generated code of course.
 


> Signed-off-by: Stefan Beller <sbeller@google.com>
> ---
> 
> Notes:
>     Also a patch, which hasn't been posted on the mailing list before.
> 
>  refs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/refs.c b/refs.c
> index b54b5b3..2942227 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -3532,7 +3532,7 @@ struct ref_update {
>  	unsigned char new_sha1[20];
>  	unsigned char old_sha1[20];
>  	int flags; /* REF_NODEREF? */
> -	int have_old; /* 1 if old_sha1 is valid, 0 otherwise */
> +	int have_old:1; /* 1 if old_sha1 is valid, 0 otherwise */
>  	struct ref_lock *lock;
>  	int type;
>  	char *msg;
> 

  reply	other threads:[~2014-12-04 16:10 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-04  8:29 [PATCHv3 00/13] the refs-transactions-reflog series Stefan Beller
2014-12-04  8:29 ` [PATCH 01/13] refs.c: make ref_transaction_create a wrapper for ref_transaction_update Stefan Beller
2014-12-04  8:29 ` [PATCH 02/13] refs.c: make ref_transaction_delete " Stefan Beller
2014-12-04  8:29 ` [PATCH 03/13] refs.c: add a function to append a reflog entry to a fd Stefan Beller
2014-12-04  8:29 ` [PATCH 04/13] refs.c: rename the transaction functions Stefan Beller
2014-12-04  8:29 ` [PATCH 05/13] refs.c: rename transaction.updates to transaction.ref_updates Stefan Beller
2014-12-04  8:29 ` [PATCH 06/13] refs.c: add a transaction function to truncate or append a reflog entry Stefan Beller
2014-12-04  8:29 ` [PATCH 07/13] reflog.c: use a reflog transaction when writing during expire Stefan Beller
2014-12-04  8:29 ` [PATCH 08/13] refs.c: rename log_ref_setup to create_reflog Stefan Beller
2014-12-04  8:29 ` [PATCH 09/13] refs.c: remove unlock_ref/close_ref/commit_ref from the refs api Stefan Beller
2014-12-04  8:29 ` [PATCH 10/13] refs.c: remove lock_any_ref_for_update Stefan Beller
2014-12-04  8:29 ` [PATCH 11/13] refs.c: don't expose the internal struct ref_lock in the header file Stefan Beller
2014-12-04  8:29 ` [PATCH 12/13] refs.c: use a bit for ref_update have_old Stefan Beller
2014-12-04 16:10   ` Torsten Bögershausen [this message]
2014-12-04 17:00   ` Andreas Schwab
2014-12-04  8:29 ` [PATCH 13/13] refs.c: allow deleting refs with a broken sha1 Stefan Beller
2014-12-04 17:10 ` [PATCHv3 00/13] the refs-transactions-reflog series Michael Haggerty
2014-12-04 17:53   ` Jonathan Nieder
2014-12-04 18:14   ` Jonathan Nieder
2014-12-04 18:32     ` Stefan Beller
2014-12-04 21:13       ` Michael Haggerty
2014-12-04 18:37   ` Junio C Hamano
2014-12-04 18:41 ` Junio C Hamano
2014-12-04 18:49   ` Stefan Beller
2014-12-04 19:27     ` Jonathan Nieder

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=5480875D.6050803@web.de \
    --to=tboegi@web.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=mhagger@alum.mit.edu \
    --cc=ronniesahlberg@gmail.com \
    --cc=sbeller@google.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.