Re: [PATCH 4/5] strmap: add strdup_strings option

From: Jeff King <peff@peff.net>
Date: Aug 28, 2020, 07:08 UTC
Message-ID: <20200828070802.GC2105050@coredump.intra.peff.net>
In-Reply-To: <CABPp-BE8tdpjx2RBGyZOYV4hsfjm5HF_dmehvX792x7TtWkLcA@mail.gmail.com>

On Fri, Aug 21, 2020 at 03:25:44PM -0700, Elijah Newren wrote:

Show 7 quoted lines

> >   - That sounds like a lot of maps. :) I guess you've looked at
> >     compacting some of them into a single map-to-struct?
> 
> Oh, map-to-struct is the primary use.  But compacting them won't work,
> because the reason for the additional maps is that they have different
> sets of keys (this set of paths meet a certain condition...).  Only
> one map contains all the paths involved in the merge.

OK, I guess I'm not surprised that you would not have missed such an obvious optimization. :)

Show 6 quoted lines

> Also, several of those maps don't even store a value; and are really
> just a set implemented via strmap (thus meaning the only bit of data I
> need for some conditions is whether any given path meets it).  It
> seems slightly ugly to have to call strmap_put(map, string, NULL) for
> those.  I wonder if I should have another strset type much like your
> suggesting for strintmap.  Hmm...

FWIW, khash does have a "set" mode where it avoids allocating the value array at all.

What's the easiest way to benchmark merge-ort? I suspect I could swap out hashmap for khash (messily) in an hour or less.

-Peff