Re: What to do with inline warnings?

"Martijn van Oosterhout" <kleptog(at)svana(dot)org> writes:

> On Wed, May 14, 2008 at 12:45:49PM -0400, Tom Lane wrote:
>> > tqual.c: In function ‘HeapTupleSatisfiesVacuum’:
>> > tqual.c:88: error: inlining failed in call to ‘SetHintBits’: call is unlikely and code size would grow
>> > tqual.c:1057: error: called from here
>> > tqual.c:88: error: inlining failed in call to ‘SetHintBits’: call is unlikely and code size would grow
>> > tqual.c:1061: error: called from here
>>
>> Hmm, it's a bit disturbing that the compiler is taking it upon itself to
>> decide that these calls are "unlikely".
>
> Perhaps would should give it some idea about how likely they'd be,
> because clearly it has no idea now.

Well they're buried in umpteen nested if/else blocks. I'm not really convinced
it's wrong actually. Each individual call site is actually quite unlikely in
the grand scheme of things.

The compiler is faced with two alternatives (or some mixture of the two).

Either a) it doesn't inline the function in which case the entire
HeapTupleSatisfiesMVCC likely fits in cpu cache and the entirety of
SetHintBits also likely fits in cache. On the other hand it incurs the
function call overhead on every call.

Or b) it inlines the function in all its myriad of call sites bloating
HeapTupleSatisfiesMVCC quite a bit. That avoids the function call overhead but
reduces the likelihood that the function remains in cache.

I think this is actually worth profiling on different architectures to make
sure we're not doing more harm than good here. Bloating HeapTupleSatisfiesMVCC
by what looks offhand to be probably easily a factor of 2 if not quite a bit
more could actually be slowing it down significantly. Using valgrind in
cachegrind mode might also be interesting.

The Linux kernel does have some macros meant to mark unlikely branches
(usually assertion failures) but I'm not sure how they work. And Gcc also has
a few optimizations which are driven by profiling data but I it doesn't sound
like this is one of them.

--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's 24x7 Postgres support!