On Tuesday, September 24th, 2024 at 7:31 AM, Alan Maguire <alan.maguire@xxxxxxxxxx> wrote: [...] > > On 20/09/2024 20:18, Eduard Zingerman wrote: > > > [...] > > > > > @@ -669,18 +692,25 @@ static int32_t btf_encoder__tag_type(struct btf_encoder *encoder, uint32_t tag_t > > > return encoder->type_id_off + tag_type; > > > } > > > > > > -static int32_t btf_encoder__add_func_proto(struct btf_encoder *encoder, struct ftype *ftype) > > > +static int32_t btf_encoder__add_func_proto(struct btf_encoder *encoder, struct ftype *ftype, struct elf_function *func) > > > > It is a bit unfortunate that we now have this "doubled" representation > > and both ftype and elf_function pointers have to be passed through > > several layers. > > Ihor (in CC) is now working on having single btf_encoder for each > > processed CU (in order to be able to sort btf's by CU names before > > merging to a single BTF for dedup, thus making reproducible builds > > cheaper). I wonder if this could be further extended to have a > > separate BTF for functions and merge only consistent functions from > > this BTF to a final BTF. > > > If I recall, Arnaldo's work ordered the DWARF CUs to be consistently > processed to allow for reproducibility. The only danger I see with the > approach of having a BTF encoder per CU is that currently we create an > ELF function table per encoder, whereas in reality all CUs are using the > same ELF. So for this approach to work I think we'd need to share ELF > representations across encoders where appropriate. Hi Alan, Arnaldo, Eduard. Yes, this is exactly the problem I ran into while experimenting last week. On vmlinux with selftests config, the tables gathered by btf_encoder__collect_symbols in btf_encoder__new take around 50Mb of space (as measured by getrusage [1]). And so having a separate copies of them for each encoder while creating an encoder for each CU is a non-starter: there are 2k+ of CUs in vmlinux. Currently this is not a big problem as the number of encoders equals to a number of worker threads for non-reproducible builds. Although it's still a couple of hundred megabytes of likely unnecessary memory usage. I talked about this with Eduard off-list, and I was going to try making these tables shared/global, as they are used mostly for search. The only write operations on these tables that Eduard and I could find is in btf_encoder__save_func, and it should be easy to add a lock there, or even use some kind of atomic write. I think at this point making ELF function/variable tables shared between BTF encoders is a good optimization by itself. However, as I understand, Eduard suggests that these tables might be unnecessary at all, if the way of checking for and merging inconsistent functions is changed. Eduard, please feel free to correct me or expand on the idea. Alan, Arnaldo, what do you think? [1] https://man7.org/linux/man-pages/man2/getrusage.2.html > > > [...]
