All of lore.kernel.org
 help / color / mirror / Atom feed
From: Eric Wong <e@80x24.org>
To: "René Scharfe" <l.s.r@web.de>
Cc: git@vger.kernel.org, "Andrzej Hunt" <andrzej@ahunt.org>,
	"Jeff King" <peff@peff.net>, "Junio C Hamano" <gitster@pobox.com>,
	"Carlo Marcelo Arenas Belón" <carenas@gmail.com>,
	"brian m. carlson" <sandals@crustytoothpaste.net>
Subject: Re: [PATCH] hash: reduce size of algo member of object ID
Date: Mon, 4 Oct 2021 07:31:42 +0000	[thread overview]
Message-ID: <20211004073142.GA25608@dcvr> (raw)
In-Reply-To: <8da7bad2-b5a8-5aef-284b-dfa4e78556a9@web.de>

René Scharfe <l.s.r@web.de> wrote:
> cf0983213c (hash: add an algo member to struct object_id, 2021-04-26)
> introduced the algo member as an int.  This increased the size of struct
> object_id by 4 bytes (12.5%) and imposed a 4-byte alignment.  Currently
> we only need to stored the values 0, 1 and 2 in it.  Let's use an
> unsigned char instead to reduce the size overhead and lift the alignment
> requirement.

I like it.  Btw, would a bitfield enum be portable enough for us?

	enum git_hash_algo algo:8;  /* or algo:2 */

I've used those in other projects, but AFAIK they were for
gcc||clang-only.

> Not sure how to measure the performance impact of this change.  The perf
> tests mentioned by cf0983213c don't show much of a difference with
> GIT_PERF_REPEAT_COUNT=10 for me:

Thanks for that info.  I'm not familiar enough with most git
internals to know if it ought to make a difference as-is.

In my experience, changes like this open the door to further
struct-packing opportunities (possibly combined with conditional
use of __attribute__((packed)) ).

  reply	other threads:[~2021-10-04  7:31 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-03  5:51 [PATCH] hash: reduce size of algo member of object ID René Scharfe
2021-10-04  7:31 ` Eric Wong [this message]
2021-10-04  8:13 ` Jeff King
2021-10-04  8:20   ` Jeff King
2021-10-05  0:04     ` brian m. carlson
2021-10-05 17:28     ` René Scharfe
2021-10-04  8:47   ` Ævar Arnfjörð Bjarmason

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=20211004073142.GA25608@dcvr \
    --to=e@80x24.org \
    --cc=andrzej@ahunt.org \
    --cc=carenas@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=l.s.r@web.de \
    --cc=peff@peff.net \
    --cc=sandals@crustytoothpaste.net \
    /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.