Re: [RFC PATCH 9/9] pahole: faster reproducible BTF encoding

[Eduard Zingerman <eddyz87@gmail.com>]

On Thu, 2024-11-28 at 01:24 +0000, Ihor Solodrai wrote:
> Change multithreaded implementation of BTF encoding:
> 
>   * Use a single btf_encoder accumulating BTF for all compilation units
>   * Make BTF encoding routine exclusive: only one thread at a time may
>     execute btf_encoder__encode_cu
>   * Introduce CU ids: an id is an index of a CU, in order they are
>     created in dwarf_loader.c
>   * Introduce CU__PROCESSED cu_state to inidicate what CUs have been
>     processed by the encoder
>   * Enforce encoding order of compilation units (struct cu) loaded
>     from DWARF by utilizing global struct cus as a queue
>   * reproducible_build option is now moot: BTF encoding is always
>     reproducible with this change
>   * Most of the code that merged the results of multiple BTF encoders
>     into one BTF after CU processing is removed
> 
> Motivation behind this change and analysis that led to it are in the
> cover letter to the patch series.
> 
> In short, this implementation of BTF encoding makes it reproducible
> without sacrificing the performance gains from parallel
> processing. The speed in terms of wall-clock time is comparable to
> non-reproducible runs on pahole/next [1]. The memory footprint is
> lower with increased number of threads.
> 
> pahole/next (12ca112):
> 
>             Performance counter stats for '/home/theihor/dev/dwarves/build/pahole -J -j24 --btf_features=encode_force,var,float,enum64,decl_tag,type_tag,optimized_func,consistent_func,decl_tag_kfuncs --btf_encode_detached=/dev/null --lang_exclude=rust /home/theihor/git/kernel.org/bpf-next/kbuild-output/.tmp_vmlinux1' (13 runs):
> 
>     50,493,244,369      cycles                                                                  ( +-  0.26% )
> 
>             1.6863 +- 0.0150 seconds time elapsed  ( +-  0.89% )
> 
> jobs 1, mem 546556 Kb, time 4.53 sec
> jobs 2, mem 599776 Kb, time 2.81 sec
> jobs 4, mem 661756 Kb, time 2.05 sec
> jobs 8, mem 764584 Kb, time 1.58 sec
> jobs 16, mem 844856 Kb, time 1.59 sec
> jobs 32, mem 1047880 Kb, time 1.69 sec
> 
> This patchset on top of pahole/next:
> 
>  Performance counter stats for '/home/theihor/dev/dwarves/build/pahole -J -j24 --btf_features=encode_force,var,float,enum64,decl_tag,type_tag,optimized_func,consistent_func,decl_tag_kfuncs --btf_encode_detached=/dev/null --lang_exclude=rust /home/theihor/git/kernel.org/bpf-next/kbuild-output/.tmp_vmlinux1' (13 runs):
> 
>     31,175,635,417      cycles                                                                  ( +-  0.22% )
> 
>            1.58644 +- 0.00501 seconds time elapsed  ( +-  0.32% )
> 
> jobs 1, mem 544780 Kb, time 4.47 sec
> jobs 2, mem 553944 Kb, time 4.68 sec
> jobs 4, mem 563352 Kb, time 2.36 sec
> jobs 8, mem 585508 Kb, time 1.73 sec
> jobs 16, mem 635212 Kb, time 1.61 sec
> jobs 32, mem 772752 Kb, time 1.59 sec
> 
> [1]: https://git.kernel.org/pub/scm/devel/pahole/pahole.git/commit/?h=next&id=12ca11281912c272f931e836b9160ee827250716
> 
> Signed-off-by: Ihor Solodrai <ihor.solodrai@pm.me>
> ---

I think this is a solid idea and a good observation,
but implementation inherits unnecessary complexity from the previous design.
There is no real need to keep single-threaded and multi-threaded modes
separate, instead:
- main thread can serve as a dedicated "collector" thread,
  waiting sequentially for CUs with ids ranging from 0 to number of cus;
- configurable number of worker threads can parse DWARF concurrently
  and put CU objects to the processing queue;
- the queue size has to be bounded to keep memory consumption within
  certain limits (but be careful, a simple bounded queue protected by
  semaphore won't do, as the queue might get fully filled with ids
  different from expected, in case when first CU takes a very long time
  to process and N CUs after it take very short time to process).

[...]