Re: [PATCH v2 1/5] build: deprecate --enable-gprof builds and remove from CI

[David Woodhouse <dwmw2@infradead.org>]

[-- Attachment #1: Type: text/plain, Size: 4900 bytes --]

On Tue, 2023-02-07 at 21:14 +0100, Thomas Huth wrote:
> From: Alex Bennée <alex.bennee@linaro.org>
> 
> As gprof relies on instrumentation you rarely get useful data compared
> to a real optimised build. Lets deprecate the build option and
> simplify the CI configuration as a result.
> 
> Buglink: https://gitlab.com/qemu-project/qemu/-/issues/1338
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Message-Id: <20230131094224.861621-1-alex.bennee@linaro.org>
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
> Signed-off-by: Thomas Huth <thuth@redhat.com>
> ---
>  docs/about/deprecated.rst  | 14 ++++++++++++++
>  meson.build                |  7 ++++++-
>  .gitlab-ci.d/buildtest.yml | 19 ++++---------------
>  meson_options.txt          |  3 ++-
>  4 files changed, 26 insertions(+), 17 deletions(-)
> 
> diff --git a/docs/about/deprecated.rst b/docs/about/deprecated.rst
> index da2e6fe63d..9317046177 100644
> --- a/docs/about/deprecated.rst
> +++ b/docs/about/deprecated.rst
> @@ -20,6 +20,20 @@ they were first deprecated in the 2.10.0 release.
>  What follows is a list of all features currently marked as
>  deprecated.
>  
> +Build options
> +-------------
> +
> +``gprof`` builds (since 8.0)
> +''''''''''''''''''''''''''''
> +
> +The ``--enable-gprof`` configure setting relies on compiler
> +instrumentation to gather its data which can distort the generated
> +profile. As other non-instrumenting tools are available that give a
> +more holistic view of the system with non-instrumented binaries we are
> +deprecating the build option and no longer defend it in CI. The

I'm not sure I understand the verb 'defend' in that sentence?

> +``--enable-gcov`` build option remains for analysis test case
> +coverage.
> +
>  System emulator command line arguments
>  --------------------------------------
>  
> diff --git a/meson.build b/meson.build
> index 4ba3bf3431..bc60bf3c4c 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -3784,7 +3784,12 @@ summary_info += {'mutex debugging':   get_option('debug_mutex')}
>  summary_info += {'memory allocator':  get_option('malloc')}
>  summary_info += {'avx2 optimization': config_host_data.get('CONFIG_AVX2_OPT')}
>  summary_info += {'avx512f optimization': config_host_data.get('CONFIG_AVX512F_OPT')}
> -summary_info += {'gprof enabled':     get_option('gprof')}
> +if get_option('gprof')
> +  gprof_info = 'YES (deprecated)'
> +else
> +  gprof_info = get_option('gprof')
> +endif
> +summary_info += {'gprof':             gprof_info}
>  summary_info += {'gcov':              get_option('b_coverage')}
>  summary_info += {'thread sanitizer':  config_host.has_key('CONFIG_TSAN')}
>  summary_info += {'CFI support':       get_option('cfi')}
> diff --git a/.gitlab-ci.d/buildtest.yml b/.gitlab-ci.d/buildtest.yml
> index 0aa149a352..8f332fc36f 100644
> --- a/.gitlab-ci.d/buildtest.yml
> +++ b/.gitlab-ci.d/buildtest.yml
> @@ -467,27 +467,16 @@ tsan-build:
>      TARGETS: x86_64-softmmu ppc64-softmmu riscv64-softmmu x86_64-linux-user
>      MAKE_CHECK_ARGS: bench V=1
>  
> -# gprof/gcov are GCC features
> -build-gprof-gcov:
> +# gcov is a GCC features

Nit: "is a GCC feature". Other than that, FWIW,

Reviewed-by: David Woodhouse <dwmw2@infradead.org>


> +gcov:
>    extends: .native_build_job_template
>    needs:
>      job: amd64-ubuntu2004-container
> +  timeout: 80m
>    variables:
>      IMAGE: ubuntu2004
> -    CONFIGURE_ARGS: --enable-gprof --enable-gcov
> +    CONFIGURE_ARGS: --enable-gcov
>      TARGETS: aarch64-softmmu ppc64-softmmu s390x-softmmu x86_64-softmmu
> -  artifacts:
> -    expire_in: 1 days
> -    paths:
> -      - build
> -
> -check-gprof-gcov:
> -  extends: .native_test_job_template
> -  needs:
> -    - job: build-gprof-gcov
> -      artifacts: true
> -  variables:
> -    IMAGE: ubuntu2004
>      MAKE_CHECK_ARGS: check
>    after_script:
>      - cd build
> diff --git a/meson_options.txt b/meson_options.txt
> index 559a571b6b..53459c15fc 100644
> --- a/meson_options.txt
> +++ b/meson_options.txt
> @@ -316,7 +316,8 @@ option('debug_stack_usage', type: 'boolean', value: false,
>  option('qom_cast_debug', type: 'boolean', value: false,
>         description: 'cast debugging support')
>  option('gprof', type: 'boolean', value: false,
> -       description: 'QEMU profiling with gprof')
> +       description: 'QEMU profiling with gprof',
> +       deprecated: true)
>  option('profiler', type: 'boolean', value: false,
>         description: 'profiler support')
>  option('slirp_smbd', type : 'feature', value : 'auto',


[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5965 bytes --]