Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1187737

Re: [PATCH v2] jhash: Deinline jhash, jhash2 and __jhash_nwords

From David Miller <davem@davemloft.net>
Newsgroups linux.kernel
Subject Re: [PATCH v2] jhash: Deinline jhash, jhash2 and __jhash_nwords
Date 2015-07-19 20:50 +0200
Message-ID <pO1OV-2C2-7@gated-at.bofh.it> (permalink)
References <pMTA7-2Br-43@gated-at.bofh.it> <pMVVg-66m-25@gated-at.bofh.it> <pNYxH-6tC-9@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


From: Denys Vlasenko <dvlasenk@redhat.com>
Date: Sun, 19 Jul 2015 17:14:53 +0200

> On 07/16/2015 08:17 PM, David Miller wrote:
>> From: Tom Herbert <tom@herbertland.com>
>> Date: Thu, 16 Jul 2015 08:43:25 -0700
>> 
>>> On Thu, Jul 16, 2015 at 5:40 AM, Denys Vlasenko <dvlasenk@redhat.com> wrote:
>>>> This patch deinlines jhash, jhash2 and __jhash_nwords.
>>>>
>>>> It also removes rhashtable_jhash2(key, length, seed)
>>>> because it was merely calling jhash2(key, length, seed).
>>>>
>>>> With this .config: http://busybox.net/~vda/kernel_config,
>>>> after deinlining these functions have sizes and callsite counts
>>>> as follows:
>>>>
>>>> __jhash_nwords: 72 bytes, 75 calls
>>>> jhash: 297 bytes, 111 calls
>>>> jhash2: 205 bytes, 136 calls
>>>>
>>> jhash is used in several places in the critical data path. Does the
>>> decrease in text size justify performance impact of not inlining it?
>> 
>> Tom took the words right out of my mouth.
>> 
>> Denys, you keep making deinlining changes like this all the time, like
>> a robot.  But I never see you make any effort to look into the performance
>> nor code generation ramifications of your changes.
> 
> The performance impact of the call/ret pair on modern x86
> CPUs is only 5 cycles. To make a real difference in execution
> speed, the function has to be less than 100 bytes of code.

What performance metrics have you collected to assert that deinlining
doesn't matter?  What networking tests have you done which stress the
socket demux path?

You still aren't addressing any of my concerns.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Back to linux.kernel | Previous | NextPrevious in thread | Find similar | Unroll thread


Thread

Re: [PATCH v2] jhash: Deinline jhash, jhash2 and __jhash_nwords Tom Herbert <tom@herbertland.com> - 2015-07-16 17:50 +0200
  Re: [PATCH v2] jhash: Deinline jhash, jhash2 and __jhash_nwords David Miller <davem@davemloft.net> - 2015-07-16 20:20 +0200
    Re: [PATCH v2] jhash: Deinline jhash, jhash2 and __jhash_nwords Joe Perches <joe@perches.com> - 2015-07-16 21:30 +0200
      Re: [PATCH v2] jhash: Deinline jhash, jhash2 and __jhash_nwords Hagen Paul Pfeifer <hagen@jauu.net> - 2015-07-17 15:50 +0200
        Re: [PATCH v2] jhash: Deinline jhash, jhash2 and __jhash_nwords Joe Perches <joe@perches.com> - 2015-07-18 01:00 +0200
    Re: [PATCH v2] jhash: Deinline jhash, jhash2 and __jhash_nwords David Miller <davem@davemloft.net> - 2015-07-19 20:50 +0200

csiph-web