On Wed, Dec 07, 2022 at 04:41:09PM -0500, Siddhesh Poyarekar wrote: > On Wed, Dec 7, 2022 at 3:15 PM Andrii Nakryiko > <andrii.nakry...@gmail.com> wrote: > > > > > On Tue, Dec 6, 2022 at 12:56 PM Andrii Nakryiko > > > <andrii.nakryiko(a)gmail.com> wrote: > > > > > > I don't think the two are comparable at all, neither in terms of > > > potential performance impact (register pressure across an entire > > > program vs at specific API call points in some unique cases) nor in > > > terms of the benefits it provides. > > > > It's irrelevant if they are comparable. This is a system-wide change that > > has potential to cause performance regression. By how much -- not clear. > > Hand-waiving such concerns is extremely disappointing in the face of the > > scrutiny given to https://pagure.io/fesco/issue/2817. So, please get > > inspiration from that proposal and do benchmarks. > > You're basically making a political statement, so I'll just leave that > bit for FESCO to deal with.
Right. I'll say right away that I will NOT ask for an additional test mass rebuild and benchmarks. The size measurements that were posted in the google docs spreadsheet are enough for me. The size change is negative for most packages, and this generally means that the execution time will not go up. (To get noticeable slowdowns the dynamic checks would need to be called often, and the instructions for a call are fairly verbose, so a large number of added runtime checks would have to be visible in the instruction count. When we get a smaller code size, this strongly implies that the compiler was able to deduce that the constrains are satisfied in more cases and actually remove checks. I know this is not a "proof", but I expect those expectation to be confirmed after we have done the mass rebuild.) There are some packages for which runtime might go up. But this is not a blocker: either they can be "fixed" to avoid the issue, or they can opt out, or we can just ignore the difference because for most programs a slowdown of 10% is completely irrelevant. So for *this* change, I think the Owner has provided enough information and justification and I'll be voting +1. We could ask the Owner to do much more benchmarking, but it wouldn't really do much except burn their time. That said, I think the same reasoning applies to -fno-omit-framepointer. The Owners of *that* change provided benchmarking that showed that the impact is negligible for most packages, and if it were to turn out that there are some packages which are noticeably negatively affected, the same fix-or-opt-out-or-ignore trifecta would be applicable. I disagree with the decision of -fno-omit-framepointer and hope we can revisit it. Nevertheless, that is not a reason to block this change. The bar was raised unreasonably high for one proposal, but instead of trying to treat the next proposal the same and block it this way, let's just return the bar to a reasonable height. Zbyszek _______________________________________________ devel mailing list -- devel@lists.fedoraproject.org To unsubscribe send an email to devel-le...@lists.fedoraproject.org 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@lists.fedoraproject.org Do not reply to spam, report it: https://pagure.io/fedora-infrastructure/new_issue
