All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: git@vger.kernel.org
Subject: Re: [RFC/PATCH 0/4] commit lists as priority queues
Date: Fri, 20 May 2011 15:03:08 -0700	[thread overview]
Message-ID: <7v4o4p2foj.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: 20110519212349.GA28589@sigill.intra.peff.net

Jeff King <peff@peff.net> writes:

> ... I also had a feeling we
> could use a faster commit_list in other places, since get_revision is
> all based around the same pop-and-push-the-parents workflow. But after
> spending a few hours on it, it seems that _lots_ of places in the code
> work on the assumption of a linked list (or at least doing very simple
> in-order traversal), and the changes got very ugly very quickly.

I recall looking at the same codepaths a few years ago for different
reasons. get_revision() callchain does a _lot_ of alloc/free, so I
naturally wanted to catch all the allocations and frees to replace them
with a custom allocator that slices from a larger slab. It involved too
many places so I decided it wasn't really worth doing only that change
without other benefit (e.g. lifting the "listness" assumption), and
stopped.

If somebody is interested to tackle it, we would probably need a two step
conversion process. First introduce an abstracted interface that passes
around the "list-as-a-whole" object with a few selected methods (e.g. drop
an element into it at the sorted location and get a "cursor" that points
at it, given a "cursor", grab an element from the list, increment the
"cursor" to point at the next object, etc.), choosing a set just
sufficient to express the patterns of accesses the current codebase makes,
and implement the API over the existing linked-list implementation to
convert all the callers. After making sure this works, update the API
implementation with the queue in this patch.

Or something like that.

> ... And
> performance-wise, it doesn't seem to be a huge problem, mainly because:
>
>   1. We don't tend to put more items in a commit_list than the width of
>      the history graph. So our "n" tends to be in the dozens at most.

I was worried that you may end up with many concurrent traversal going in
a very bushy history (think: traversing from the tip of "next"), but I
agree that it would be in the order of dozens, not thousands.

  parent reply	other threads:[~2011-05-20 22:03 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2011-05-19 20:48 [PATCH] fetch: avoid repeated commits in mark_complete Jeff King
2011-05-19 21:23 ` [RFC/PATCH 0/4] commit lists as priority queues Jeff King
2011-05-19 21:24   ` [PATCH 1/4] Makefile: sort TEST_PROGRAMS list Jeff King
2011-05-19 21:24   ` [PATCH 2/4] basic priority queue implementation Jeff King
2011-05-20  0:47     ` Thiago Farina
2011-05-20  7:38       ` Jeff King
2011-05-20 13:13         ` Thiago Farina
2011-05-20 13:23           ` Jeff King
2011-05-19 21:25   ` [PATCH 3/4] commit: add infrastructure for priority queues of commits Jeff King
2011-05-19 21:26   ` [PATCH 4/4] fetch-pack: use priority queue for mark_complete Jeff King
2011-05-20 22:03   ` Junio C Hamano [this message]
2011-05-20  1:42 ` [PATCH] fetch: avoid repeated commits in mark_complete Junio C Hamano

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=7v4o4p2foj.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --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.