All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Michael Haggerty <mhagger@alum.mit.edu>,
	git discussion list <git@vger.kernel.org>
Subject: Re: [PATCH 2/2] log: do not shorten decoration names too early
Date: Thu, 14 May 2015 11:01:03 -0700	[thread overview]
Message-ID: <xmqqzj576nts.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20150514174945.GB7966@peff.net> (Jeff King's message of "Thu, 14 May 2015 13:49:45 -0400")

Jeff King <peff@peff.net> writes:

> On Thu, May 14, 2015 at 10:37:39AM -0700, Junio C Hamano wrote:
> ...
>> Wouldn't the hashtable used by decorate.[ch] with the max load
>> factor capped to 66% a better economy?
>
> Good point. A slab would be less memory efficient, but cheaper to look
> up (it is a direct memory reference, with no probing and no hashcmp()).
> But cache effects matter, so it could even be slower.

Yes, that was what I meant by economy.  I do not think memory footprint
is free in that sense.

> On the other hand, the slab makes it easy to actually store the real
> type (struct name_decoration), whereas the decorate hash stores only
> pointers. So we'd save an extra malloc/pointer in each case.
>
> So with your slab_peek() below, I'd guess that the slab would actually
> be faster. But I'd also be unsurprised if it makes no appreciable
> difference to the overall runtime of "git log --decorate". I think we'd
> have to build it and profile (and please feel free to say "eh, not worth
> the time to think about further").

While I think *slabname##_peek() would be worth doing regardless of
this caller, I suspect that the major overhead of decorate code
would come from the for_each_ref() that jumps deep into the history
to parse old commits; it would trigger a lot of unpacking of objects
deep in the delta chain, which would be expensive than table look-up
in either scheme.

>> I notice that there is no API into commit_slab to ask "Does this
>> commit have data in the slab?"  *slabname##_at() may be the closest
>> thing, but that would allocate the space and then says "This is the
>> slot for that commit; go check if there is data there already."
>
> Yes. I think it's not a big deal if your slab is reasonably full (you'll
> extend it to the full size of your commit space eventually either way).
> But for a sparse slab, it does make that query much more expensive than
> it needs to be.

Yes, and I think that commit decoration is such a use case.

  reply	other threads:[~2015-05-14 18:01 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-05-13 13:11 "HEAD -> branch" decoration doesn't work with "--decorate=full" Michael Haggerty
2015-05-13 14:51 ` Junio C Hamano
2015-05-13 15:26   ` Michael J Gruber
2015-05-13 17:11 ` Junio C Hamano
2015-05-13 17:13   ` Junio C Hamano
2015-05-13 19:40     ` [PATCH 2/2] log: do not shorten decoration names too early Junio C Hamano
2015-05-14  6:33       ` Jeff King
2015-05-14 17:37         ` Junio C Hamano
2015-05-14 17:49           ` Jeff King
2015-05-14 18:01             ` Junio C Hamano [this message]
2015-05-14 18:10               ` Jeff King
2015-05-14 21:49           ` Junio C Hamano
2015-05-14 21:54             ` Jeff King
2015-05-14 22:25               ` Junio C Hamano
2015-05-14 22:33                 ` Jeff King
2015-05-22 21:21                   ` Junio C Hamano
2015-05-22 21:38                     ` Jeff King

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=xmqqzj576nts.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=mhagger@alum.mit.edu \
    --cc=peff@peff.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.