All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Kristofer Karlsson via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, "René Scharfe" <l.s.r@web.de>,
	"Kristofer Karlsson" <krka@spotify.com>
Subject: Re: [PATCH v4 0/2] prio-queue: fold lazy_queue into prio_queue for automatic get+put fusion
Date: Tue, 30 Jun 2026 13:59:33 -0700	[thread overview]
Message-ID: <xmqqh5mjrbgq.fsf@gitster.g> (raw)
In-Reply-To: <pull.2140.v4.git.1780945851.gitgitgadget@gmail.com> (Kristofer Karlsson via GitGitGadget's message of "Mon, 08 Jun 2026 19:10:49 +0000")

"Kristofer Karlsson via GitGitGadget" <gitgitgadget@gmail.com>
writes:

> Rene's lazy_queue wrapper in describe.c was a clever optimization -- by
> deferring the get, a following put becomes a simple replace, avoiding a full
> remove-rebalance-insert cycle.
>
> It turns out this pattern is so common in git's traversal code that it makes
> sense to fold it into prio_queue itself. Gets and puts are interleaved in
> virtually every commit walk, so the fusion is essentially always a win.
>
> This is mostly a code simplification -- three callers had independently
> reimplemented the same optimization, and they all collapse to plain get+put
> now. The 1.7-2.7% speedup on traversal-heavy workloads is a nice bonus.
>
> More details and benchmark numbers in the commit message.
>
> Related to but independent of the cascade sift-down work in
> kk/prio-queue-cascade-sift -- the two can land in either order.
>
> Changes in v4:
>
>  * Thanks Junio for review, applied all suggestions.
>
>  * Renamed .nr_internal to .nr_
>
>  * Restored flush_get() as a static inline helper instead of inlining the
>    flush logic into get() and peek().
>
>  * Guard empty-queue check with nr_ <= get_pending.
>
>  * Flipped commit order: the rename/accessor commit is now first, and the
>    behavioral fusion change is second. This was partly messy -- the first
>    rename commit introduces some ugly intermediate code (e.g. describe.c's
>    prio_queue_for_each with a skip variable) that gets cleaned up in commit
>    2 when the lazy get makes it unnecessary.

So, this is the "other" topic that we would want to merge first
before the kk/prio-queue-cascade-sift topic.  This round looks good
to me.

Thanks.

  parent reply	other threads:[~2026-06-30 20:59 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-06 14:58 [PATCH] prio-queue: fold lazy_queue into prio_queue for automatic get+put fusion Kristofer Karlsson via GitGitGadget
2026-06-06 16:31 ` Junio C Hamano
2026-06-06 17:24   ` Kristofer Karlsson
2026-06-08 14:07     ` Junio C Hamano
2026-06-06 19:01 ` [PATCH v2 0/2] " Kristofer Karlsson via GitGitGadget
2026-06-06 19:01   ` [PATCH v2 1/2] " Kristofer Karlsson via GitGitGadget
2026-06-06 19:01   ` [PATCH v2 2/2] prio-queue: rename .nr to .nr_internal to prevent direct access Kristofer Karlsson via GitGitGadget
2026-06-07  7:30   ` [PATCH v2 0/2] prio-queue: fold lazy_queue into prio_queue for automatic get+put fusion René Scharfe
2026-06-07  9:30     ` Kristofer Karlsson
2026-06-07 11:43   ` [PATCH v3 " Kristofer Karlsson via GitGitGadget
2026-06-07 11:43     ` [PATCH v3 1/2] " Kristofer Karlsson via GitGitGadget
2026-06-07 11:43     ` [PATCH v3 2/2] prio-queue: rename .nr to .nr_internal to prevent direct access Kristofer Karlsson via GitGitGadget
2026-06-08 13:36     ` [PATCH v3 0/2] prio-queue: fold lazy_queue into prio_queue for automatic get+put fusion Junio C Hamano
2026-06-08 19:10     ` [PATCH v4 " Kristofer Karlsson via GitGitGadget
2026-06-08 19:10       ` [PATCH v4 1/2] prio-queue: rename .nr to .nr_ and add accessor helpers Kristofer Karlsson via GitGitGadget
2026-06-08 19:10       ` [PATCH v4 2/2] prio-queue: fold lazy_queue into prio_queue for automatic get+put fusion Kristofer Karlsson via GitGitGadget
2026-06-30 20:59       ` Junio C Hamano [this message]
2026-06-30 21:16         ` [PATCH v4 0/2] " Kristofer Karlsson

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=xmqqh5mjrbgq.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=krka@spotify.com \
    --cc=l.s.r@web.de \
    /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.