On Fri, Mar 29, 2024 at 02:42:49PM +0100, Morten Brørup wrote: > +CC techboard > > > From: Maxime Coquelin [mailto:maxime.coque...@redhat.com] > > Sent: Friday, 29 March 2024 14.05 > > > > Hi Stephen, > > > > On 3/29/24 03:53, Stephen Hemminger wrote: > > > On Thu, 28 Mar 2024 17:10:42 -0700 > > > Andrey Ignatov <r...@apple.com> wrote: > > > > > >>> > > >>> You don't need always inline, the compiler will do it anyway. > > >> > > >> I can remove it in v2, but it's not completely obvious to me how is > > it > > >> decided when to specify it explicitly and when not? > > >> > > >> I see plenty of __rte_always_inline in this file: > > >> > > >> % git grep -c '^static __rte_always_inline' lib/vhost/virtio_net.c > > >> lib/vhost/virtio_net.c:66 > > > > > > > > > Cargo cult really. > > > > > > > Cargo cult... really? > > > > Well, I just did a quick test by comparing IO forwarding with testpmd > > between main branch and with adding a patch that removes all the > > inline/noinline in lib/vhost/virtio_net.c [0]. > > > > main branch: 14.63Mpps > > main branch - inline/noinline: 10.24Mpps > > Thank you for testing this, Maxime. Very interesting! > > It is sometimes suggested on techboard meetings that we should convert more > inline functions to non-inline for improved API/ABI stability, with the > argument that the performance of inlining is negligible.
removing inline functions probably has an even more profound negative impact when using dynamic linking. for all the value of msvc's dll scoped security features they do have overheads per-call that can't be wished away i imagine equivalents in gcc are the same. > > I think this test proves that the sum of many small (negligible) performance > differences it not negligible! sure looks that way, though i think there is some distinction to be made between inline and *forced* inline. force inline may be losing us some opportunity for the compiler to optimize better than is obvious to us. > > > > > Andrey, thanks for the patch, I'll have a look at it next week. > > > > Maxime > > > > [0]: https://pastebin.com/72P2npZ0 >
