git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
From: Jeff King <peff@peff.net>
To: Junio C Hamano <gitster@pobox.com>
Cc: Christian Couder <christian.couder@gmail.com>,
	git@vger.kernel.org, Ævar Arnfjörð Bjarmason <avarab@gmail.com>,
	Jonathan Tan <jonathantanmy@google.com>,
	SZEDER Gábor <szeder.dev@gmail.com>,
	Derrick Stolee <stolee@gmail.com>,
	Christian Couder <chriscool@tuxfamily.org>
Subject: Re: [RFC PATCH 0/5] oidmap: handle entries with the same key
Date: Fri, 12 Jul 2019 16:58:16 -0500
Message-ID: <20190712215815.GB3593@sigill.intra.peff.net> (raw)
In-Reply-To: <xmqqlfx3p0zo.fsf@gitster-ct.c.googlers.com>

On Fri, Jul 12, 2019 at 11:21:47AM -0700, Junio C Hamano wrote:

> > Christian Couder <christian.couder@gmail.com> writes:
> >
> >> This is an RFC patch series that is not intended to be merged for now,
> >> as it looks like we don't need oidmaps that can handle several entries
> >> with the same key yet.
> >
> > What does it even mean for a map to allow multiple entries per key?
> 
> Ah, one thing that I was missing (perhaps it was obvious to
> everybody else but me X-<) was that this is merely to expose what is
> already available in the underlying hashmap API, so let's not bother
> with the "don't people usually do a single key to a value, which
> happens to be a bag of stuff (not just a single stuff)?" question.
> 
> And from that "a generic hashmap can do this, and an upcoming code
> needs to use a hashmap keyed with oid in the same fashion" point of
> view, the new wrappers the patches add all made sense to me.

FWIW, I went through the same thought process. :)

One devil's advocate point against, though: we found recently that khash
performs much better than hashmap.[ch] for the oidset data structure.
AFAIK nobody has looked at whether the same is true for oidmap. But if
it is, then this strategy may make it harder to switch.

(OTOH, we already have kh_oid_map, so the two could probably co-exist
and we could just convert particular callers from one to the other).

> > I actually think that showing how it is used in the real application
> > (reftable?) is the best way to illustrate why this is useful and to
> > get opinions from others.
> 
> This part still stands, though.

Agreed.

-Peff

      reply index

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-07  8:29 Christian Couder
2019-07-07  8:29 ` [RFC PATCH 1/5] oidmap: add oidmap_add() Christian Couder
2019-07-07  8:29 ` [RFC PATCH 2/5] oidmap: add oidmap_get_next() Christian Couder
2019-07-07  8:30 ` [RFC PATCH 3/5] test-oidmap: add back proper 'add' subcommand Christian Couder
2019-07-07  8:30 ` [RFC PATCH 4/5] test-oidmap: add 'get_all' subcommand Christian Couder
2019-07-07  8:30 ` [RFC PATCH 5/5] t0016: add 'add' and 'get_all' subcommand test Christian Couder
2019-07-08 20:24 ` [RFC PATCH 0/5] oidmap: handle entries with the same key Junio C Hamano
2019-07-12 18:21   ` Junio C Hamano
2019-07-12 21:58     ` Jeff King [this message]

Reply instructions:

You may reply publically 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

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190712215815.GB3593@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=avarab@gmail.com \
    --cc=chriscool@tuxfamily.org \
    --cc=christian.couder@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jonathantanmy@google.com \
    --cc=stolee@gmail.com \
    --cc=szeder.dev@gmail.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

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror http://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox