On Tue, Sep 24, 2019 at 07:03:50PM +0200, René Scharfe wrote: > Am 23.09.19 um 22:47 schrieb SZEDER Gábor: > > On Mon, Sep 23, 2019 at 09:55:11PM +0200, René Scharfe wrote: > >> -- >8 -- > >> Subject: [PATCH] name-rev: use FLEX_ARRAY for tip_name in struct rev_name > >> > >> Give each rev_name its very own tip_name string. This simplifies memory > >> ownership, as callers of name_rev() only have to make sure the tip_name > >> parameter exists for the duration of the call and don't have to preserve > >> it for the whole run of the program. > >> > >> It also saves four or eight bytes per object because this change removes > >> the pointer indirection. Memory usage is still higher for linear > >> histories that previously shared the same tip_name value between > >> multiple name_rev instances. > > > > Besides looking at memory usage, have you run any performance > > benchmarks? Here it seems to make 'git name-rev --all >out' slower by > > 17% in the git repo and by 19.5% in the linux repo. > > Did measure now; I also see a slowdown with my patch applied:
Thanks for confirming. > We can reuse a strbuf instead of allocating new strings when adding > suffixes to get some of the performance loss back. I guess it's easier > after the recursion is removed. Numbers: Agreed, the conflicts on first sight are too ugly to have these changes in parallel cooking topics. Furthermore, after the recursion is gone we can measure the memory usage and performance impact of your changes even in big linear repositories. I think I will drop the last two patches plugging memory leaks from v2 of my series, because it seems your proposed changes do it cleaner and make them moot anyway.
