Deprecating --enable-gprof?

Deprecating --enable-gprof?

[Marc-André Lureau]

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

Hi

configure has --enable-gprof since its creation, but is it actually being
used, or is it sporadic enough that we could deprecate it?

I have some reason to believe that it's not used much:
- --enable-gprof only enables -p, which is prof (not gprof!)
- nowadays there are better profiling tools, such as perf

Should we fix it to use gprof instead? Or does anyone actually care about
it?

It is problematic as the flag is passed to meson globally
(add_project_arguments), but some targets may not support it. I have
pending patches for pc-bios/* roms. And we can't remove the flag for those
easily (not as easily as if it was supported by meson, like coverage)

I would propose to deprecate it on the configure options. A user can always
override the cflags manually to provide -p option if he really needs it.
Alternatively, I could work on getting support in meson (
https://github.com/mesonbuild/meson/issues/3659), but this would bump our
meson dependency more.

What do you think?

-- 
Marc-André Lureau

[-- Attachment #2: Type: text/html, Size: 1389 bytes --]

Re: Deprecating --enable-gprof?

[Daniel P. Berrangé]

On Fri, Sep 18, 2020 at 05:25:45PM +0400, Marc-André Lureau wrote:
> Hi
> 
> configure has --enable-gprof since its creation, but is it actually being
> used, or is it sporadic enough that we could deprecate it?
> 
> I have some reason to believe that it's not used much:
> - --enable-gprof only enables -p, which is prof (not gprof!)
> - nowadays there are better profiling tools, such as perf
> 
> Should we fix it to use gprof instead? Or does anyone actually care about
> it?

If we consider a future where configure is eliminated, then I think
the answer is it should be made a built-in feature of meson.

Whether we want to remove it from configure now, in anticipation
of this future day though...

> It is problematic as the flag is passed to meson globally
> (add_project_arguments), but some targets may not support it. I have
> pending patches for pc-bios/* roms. And we can't remove the flag for those
> easily (not as easily as if it was supported by meson, like coverage)

Is that just because it gets added to QEMU_CFLAGS instead of having
it own explicit config option ? If it doesn't work for some parts
of the build, then we can we just add a GPROF_CFLAGS arg and only
add it to the parts of the build where it actually works ?


> 
> I would propose to deprecate it on the configure options. A user can always
> override the cflags manually to provide -p option if he really needs it.
> Alternatively, I could work on getting support in meson (
> https://github.com/mesonbuild/meson/issues/3659), but this would bump our
> meson dependency more.

Meson has built-in support for various similar concepts such as
coverage, address sanitizer, profile guided optimization and so
on. It seems like grof fits in meson in this way.

We would not need to bump our minimum depedancy if it merely
requires a meson flag to be passed - anyone who wanted to use
it would merely install newer meson to get access to the flag.
We're not using it by default, so we don't have a need to bump
the version.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: Deprecating --enable-gprof?

[Peter Maydell]

On Fri, 18 Sep 2020 at 14:27, Marc-André Lureau
<marcandre.lureau@gmail.com> wrote:
>
> Hi
>
> configure has --enable-gprof since its creation, but is it actually being used, or is it sporadic enough that we could deprecate it?

This reminds me (because I can never keep gprof and gcov straight
in my head :-)) that now the meson build has landed we should
probably insist on gcov/gcovr being at least version 4.1, because
older versions don't correctly handle out of tree builds:
https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg05823.html

-- PMM

Re: Deprecating --enable-gprof?

[Daniel P. Berrangé]

On Fri, Sep 18, 2020 at 03:19:00PM +0100, Peter Maydell wrote:
> On Fri, 18 Sep 2020 at 14:27, Marc-André Lureau
> <marcandre.lureau@gmail.com> wrote:
> >
> > Hi
> >
> > configure has --enable-gprof since its creation, but is it actually being used, or is it sporadic enough that we could deprecate it?
> 
> This reminds me (because I can never keep gprof and gcov straight
> in my head :-)) that now the meson build has landed we should
> probably insist on gcov/gcovr being at least version 4.1, because
> older versions don't correctly handle out of tree builds:
> https://lists.gnu.org/archive/html/qemu-devel/2020-06/msg05823.html

I think we might not need todo anything at this point. --enable-gcov
just delegates to meson by passing  "-Db_coverage=true" to meson.

We still have a coverage-summary.sh script that is run in Travis, but
I'm not sure if that's obsolete or not

If we were using meson/ninja directly we'd just do "meson coverage-html"
to generate a report:

  https://mesonbuild.com/howtox.html#producing-a-coverage-report

and the resulting file could be published from CI as an artifact.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: Deprecating --enable-gprof?

[Paolo Bonzini]

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

Il ven 18 set 2020, 16:27 Daniel P. Berrangé <berrange@redhat.com> ha
scritto:

> If we were using meson/ninja directly we'd just do "meson coverage-html"
> to generate a report:
>
>   https://mesonbuild.com/howtox.html#producing-a-coverage-report


Note that it's "meson compile coverage-html", i.e. "ninja coverage-html".
So we ought to be able to do just "make coverage-html".

Paolo


>
> and the resulting file could be published from CI as an artifact.
>
> Regards,
> Daniel
> --
> |: https://berrange.com      -o-
> https://www.flickr.com/photos/dberrange :|
> |: https://libvirt.org         -o-
> https://fstop138.berrange.com :|
> |: https://entangle-photo.org    -o-
> https://www.instagram.com/dberrange :|
>
>

[-- Attachment #2: Type: text/html, Size: 2052 bytes --]