On Fri, Oct 30, 2020 at 7:56 AM Jeff King <peff@xxxxxxxx> wrote: > > On Tue, Oct 13, 2020 at 12:40:50AM +0000, Elijah Newren via GitGitGadget wrote: > > > For heavy users of strmaps, allowing the keys and entries to be > > allocated from a memory pool can provide significant overhead savings. > > Add an option to strmap_ocd_init() to specify a memory pool. > > So this one interacts badly with my FLEXPTR suggestion. > > I guess it provides most of the benefit that FLEXPTR would, because > we're getting both the entries and the strings from the mempool. Which > really ends up being an almost identical memory layout, since the > mempool presumably just gives you the N bytes for the string right after > the last thing you allocated, which would be the struct. > > The only downside is that if you don't want to use the mempool (e.g., > because you might actually strmap_remove() things), you don't get the > advantage. > > I think we could fall back to a FLEXPTR when there's no mempool (or even > when there is, though you'd be on your own to reimplement the > computation parts of FLEXPTR_ALLOC). I'm not sure how ugly it would end > up. Yeah, we'd need a mempool-specific reimplementation of FLEXPTR_ALLOC with the mempool, and just avoid using it at all whenever strdup_strings was 0. Seems slightly ugly, but maybe it wouldn't be too bad. I could look into it. > I haven't used our mem_pool before, but the code all looks quite > straightforward to me. I guess the caller is responsible for > de-allocating the mempool, which makes sense. It would be nice to see > real numbers on how much this helps, but again, you might not have the > commits in the right order to easily find out. At the time I implemented it, I did grab some numbers. It varied quite a bit between different cases, since a lot of my strmaps are for tracking when special cases arise and we can implement various optimizations. Naturally, a usecase which involves heavier use of strmaps will mean greater benefits from using a mempool. Also, if I had implemented it later, after one rename-related optimization I hadn't yet discovered at the time, then it would have shown a larger relative reduction in overall execution time. Anyway, at the time I put the mempool into strmaps and made use of it in relevant places, one of my rebase testcases saw an almost 5% reduction in overall execution time. I'm sure it would have been over 5% if I had reordered it to come after my final rename optimization.
