On Fri, Jan 06, 2023 at 10:24:39AM +0000, Jonathan Wakely wrote: > But they should be measurable, right? If profiling can't actually > measure performance and track improvements in performance, the change > isn't useful. So it should be possible to show the benefits over the > next release or two. The problem is you're confusing general gains and gains in specific scenarios. Perf + flamegraphs are such a useful tool that we managed to double performance (ie. ~ 100% gain) in one particular network server case that we investigated a few years ago. This was by spotting that the kernel was writing to an MSR (hardware register) which was really slow, and as it wasn't necessary we just got rid of it. For that one use case - an incredible performance gain! Does this mean everyone sees their machines double in speed? Of course not. The thing is that perf + flamegraphs makes your whole system much more visible and so it's much easier to find these kind of gains in specific scenarios. Frame pointers need to be enabled across the whole system for this to work properly[1]. Will we be able to say that "Fedora got N% faster" in two years? Not at all - it depends entirely what you use Fedora for. The overhead is also a real thing. There's a few percent overhead everywhere for enabling frame pointers because every stack frame entry and exit involves a couple of extra instructions. Anyway I'd really urge you to play with these tools before judging this proposal: https://www.brendangregg.com/flamegraphs.html > Aside: could the change proposal please be updated to show *how* to > opt out, not just state it can be done trivially? While this should be documented, please don't do it just because you don't like it. It makes profiling worse for everyone. Rich. [1] Perf claims to be able to use DWARF info for stack traces, but in our experience it does not work at all. > I shouldn't have to find > https://src.fedoraproject.org/rpms/redhat-rpm-config/pull-request/231#request_diff > to know whether the right opt-out is to %undefine the new macro, or to > define it to 0, or something else. > > > > > > > There needs to be substance behind such predictions if they are going > > > to be used as justification for slowing things down in the mean time. > > > > I agree. > > -- > > Sincerely, > > Demi Marie Obenour (she/her/hers) > > _______________________________________________ > > devel mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxx > > To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxx > > Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ > > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines > > List Archives: https://lists.fedoraproject.org/archives/list/devel@xxxxxxxxxxxxxxxxxxxxxxx > > Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue > _______________________________________________ > devel mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxx > To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxx > Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ > List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines > List Archives: https://lists.fedoraproject.org/archives/list/devel@xxxxxxxxxxxxxxxxxxxxxxx > Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com nbdkit - Flexible, fast NBD server with plugins https://gitlab.com/nbdkit/nbdkit _______________________________________________ devel mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://docs.fedoraproject.org/en-US/project/code-of-conduct/ List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/devel@xxxxxxxxxxxxxxxxxxxxxxx Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
