On Fri, Oct 04, 2024 at 10:15:56AM +0100, Alan Maguire wrote: > On 04/10/2024 06:13, Ihor Solodrai wrote: > > This patchset changes how BTF encoders collect and store function > > information in a table searcheable by name, where their > > btf_encoder_function_state is maintained. > > > > hi Ihor > > I'll take a more detailed look shortly, but I'd suggest respinning the > series against the updated "next" branch shortly as there are changes in > flight that Stephen is working on to encode variables; they > significantly change the processing of variables, see [1] for details. > They have partly landed and the rest will likely land soon. It might > make things easier to wait until those changes land and respin, and I > don't think they invalidate the approach you suggest, just will require > some merging around variable handling I think. Right, and this is something I was planning to work into as well: to move things from the encoders to the loaders, i.e. collecting info from the system should be the work of loaders, encoders then write things in the desired output format. This comes from discussions we had at LPC and even before, i.e. that we should be able to write BTF in the BTF encoder from info we read from BTF info, be it generated by a compiler or by pahole itself, to amend the BTF, add/remove BTF features, etc. And for that the DWARF loader should pick the things needed for encoding, say, percpu info, but when reading from a BTF file (detached or in an ELF section) we probably have that info already. I'm now waiting for Stephen to publish a new version of his patchset to review/apply/test. Then Ihor can work from there, I think. - Arnaldo > [1] https://lore.kernel.org/dwarves/8734ldm72u.fsf@xxxxxxxxxx/ > > > > Previously a separate array of elf_function structs was built and > > stored by each btf_encoder, and when the encoders were merged before > > dumping BTF their function tables would be traversed and merged too. > > > > With these changes, the table used for searching for elf_function > > becomes shared between encoder threads. However, additional > > information in the table and in each individual elf_function has to be > > stored to accomodate the sharing. > > > > The memory footprint is now slightly higher, although it degrades > > faster when number of threads is increased without this patchset. > > > > /usr/bin/time -v with -j8: > > > > With patchset: Maximum resident set size (kbytes): 1402792 (~2% higher) > > Without: Maximum resident set size (kbytes): 1376700 > > > > /usr/bin/time -v with -j16: > > > > With patchset: Maximum resident set size (kbytes): 1456044 > > Without: Maximum resident set size (kbytes): 1450748 > > > > The idea to make function table shared between encoders has been > > mentioned previously [1]. > > > > I've been looking into ways of improving parallel reproducible_build > > time, ideally making it as fast as non-reproducible. In an off-list > > discussion Eduard Zingerman suggested creating a btf_encoder for each > > input compilation unit. This would allow to enforce the order of BTF > > merge after btf_encoders are done. > > > > With each encoder maintaining it's own table this idea proved > > infeasible [2]. And so making it shared became a requirement. > > > > I implemented a prototype that improved the speed of parallel > > reproducible_build by using a similar shared table and implementing > > Eduard's idea [3]. On master (84ab453) reproducible_build was ~50% > > slower than non-reproducible, and with my experimental changes it was > > 20% slower. > > > > This patchset stems from that work and discussions. The longer term > > goal is to make BTF encoding reproducible by default, and fast. > > > > [1] https://lore.kernel.org/dwarves/20240909155031.1596249-1-alan.maguire@xxxxxxxxxx/ > > [2] https://lore.kernel.org/dwarves/aQtEzThpIhRiwQpkgeZepPGr5Mt_zuGfPbxQxZgS6SSoPYoqM1afjDqEmIZkdH3YzvhWmWwqCS_8ZvFTcSZHvpkAeBpLRTAZEmrOhq0svfo=@pm.me/ > > [3] https://github.com/theihor/dwarves/pull/1/files > > > > Ihor Solodrai (6): > > dwarf_loader: introduce pre/post cus__load_module hooks to conf_load > > btf_encoder: split btf_encoder__collect_symbols() > > btf_encoder: introduce elf_functions struct type > > btf_encoder: collect elf_functions in pre_cus__load_module > > btf_encoder: introduce elf_functions_entry > > btf_encoder: switch to shared elf_functions table > > > > btf_encoder.c | 341 ++++++++++++++++++++++++++++++++++++------------- > > btf_encoder.h | 5 +- > > dwarf_loader.c | 21 ++- > > dwarves.h | 18 ++- > > pahole.c | 28 ++-- > > 5 files changed, 303 insertions(+), 110 deletions(-) > >
