On Tue, Dec 17, 2024 at 04:40:40PM -0800, Eduard Zingerman wrote: > On Tue, 2024-12-17 at 16:03 -0800, Andrii Nakryiko wrote: > > [...] > > > I agree with Ihor. I think he invested a lot of time into these > > improvements, and asking him to re-do the series just to shuffle a few > > patches around is just an unnecessary overhead (which also delays the > > ultimate outcome: faster BTF generation with pahole). > > Patches #1-4 are good refactorings. > Patches #5-7 introduce a global state and complication where this > could be avoided. (These were necessary before Ihor figured out the > trick with patch #10). > E.g. 'elf_functions_list': > - there is no reason for it to be global, it could be a part of the > encoder, as it is now; > - there is no reason for it to be a list, encoding works with a single > function table and keeping it as a list just confuses reader. agree, with just single encoder object I don't see a reason for the btf_encoding_context, keeping functions in encoder will be simpler thanks, jirka > > Same for btf_encoder_list_lock / btf_encoder_list. > > > And as Ihor mentioned, we might improve upon this series by > > parallelizing encoders to gain some further improvements, so I think > > all the internal refactoring and preparations are setting up a good > > base for further work. > > Not really, the main insight found by Ihor is that parallel BTF > encoding doesn't make much sense: constructing BTF is as cheap as > copying it. I don't see much room for improvement here. > > [...] > >