On 2018-01-05 00:11:19 +1300, Thomas Munro wrote: > On Tue, Jan 2, 2018 at 4:58 PM, Thomas Munro > <thomas.mu...@enterprisedb.com> wrote: > > On Tue, Jan 2, 2018 at 4:17 PM, Tom Lane <t...@sss.pgh.pa.us> wrote: > >> gaur | nodeHashjoin.c:167: warning: `always_inline' attribute > >> directive ignored > >> mastodon | .\src\backend\executor\nodeHashjoin.c(165): warning C4141: > >> 'inline' : used more than once > > > > 1. MSVC doesn't like you to say both "__forceinline" and "inline". > > > > 2. GCC 2.95.3 doesn't understand always_inline. From a quick look at > > archived manuals, it seems that that attribute arrived in 3.1. > > Here is one way to fix those warnings. Thoughts?
That looks good to me. > >> Therefore, I think that pg_attribute_always_inline is not merely > >> useless but actively bad, and should be removed. > > How about a macro PG_NOINLINE, which, if defined, inhibits this? Or I > could give up this strategy and maintain two separate very similar > functions, ExecHashJoin and ExecParallelHashJoin. Unless this pg_attribute_always_inline gets a lot more widely proliferated I don't see a need to change anything. Debuggability isn't meaningfully impacted by seing more runtime attributed to ExecHashJoin/ExecParallelHashJoin rather than ExecHashJoinImpl. If this were used on functions that are called from a lot of places that argument would hold some weight, but for now I'm really not seing it. - Andres
