Re: Visibility map and hint bits

On Thu, May 5, 2011 at 2:20 PM, Merlin Moncure <mmoncure(at)gmail(dot)com> wrote:
> On Thu, May 5, 2011 at 2:00 PM, Kevin Grittner
> <Kevin(dot)Grittner(at)wicourts(dot)gov> wrote:
>> Merlin Moncure <mmoncure(at)gmail(dot)com> wrote:
>>
>>> a small cache that remembers the commit/cancel status of recently
>>> seen transactions.
>>
>> How is that different from the head of the clog SLRU?
>
> several things:
> *) any slru access requires lock (besides the lock itself, you are
> spending cycles in critical path)
> *) cache access happens at different stage of processing in
> HeapTupleSatisfiesMVCC: both TransactionIdIsCurrentTransactionId and
> TransactionIdIsInProgress have to be checked first. Logically, it's
> extension of hint bit check itself, not expansion of lower levels of
> caching
> *) in tqual.c you can sneak in some small optimizations like only
> caching the bit if it's known good in the WAL (XlogNeedsFlush).  That
> way you don't need to keep checking it over and over for the same
> trasaction
> *) slru level accesses happen too late to give much benefit:
>
> I can't stress enough how tight HeapTupleSatisfiesMVCC is.  On my
> workstation VM, each non inline function call shows up measurably in
> profiling.  I think anything you do here has to be inline, hand
> rolled, and very tight (you can forget anything around dynahash).
> Delegating the cache management to transam or (even worse) slru level
> penalizes some workloads non-trivially.

An updated patch is attached. It's still WIP, but I need a little
guidance before going further.

What I did:
*) Added a lot of source level comments that should explain better
what's happening and why
*) Fixed a significant number of goofs in the earlier patch.
*) Reorganized the interaction with HeapTupleSatisfiesMVCC. In
particular SetHintBits() is returning if it actually set the bit
because I can use that information.

What's not done:
*) Only commit bits are cached, and caching action is only happening
in HeapTupleSatisfiesMVCC. I'm not sure yet if it's better to store
invalid bits in the same cache or in a separate one. I'm not sure if
the other satisfies routines should also be engaging the cache.
Translated from nerd speak, that means I haven't yet done the research
to see when they are fired and if they are bottlenecks :-).

*) I'd like to reach some sort of consensus with Tom if there is any
point in going further in direction. Not so much on how the mechanics
of how the cache work, but that it is at the tqual.c level and the
changes to HeapTuplesSatisfiesMVCC. In particular. I think caching at
transam.c level is a dead end on performance grounds regardless of how
you implement the cache.

Some points of note:
*) Is it acceptable to use static definition of memory like that. If
not, should there be a more standard allocation under
CacheMemoryContext?

*) Testing for the benefit is simple: just create a bunch of records
and seqscan the table (select count(*)). Without the patch the first
scan is slower and does a bunch of i/o. With it, it does not.

*) The cache overhead is *almost* not measurable. As best I can tell
we are looking at maybe 1% ish overhead in synthetic scan heavy
workloads (i think this is a fair price to pay for all the i/o
savings). The degenerate case of repeated 'rollups' is really
difficult to generate, even synthetically -- if the cache is
performing lousily the regular hint bit action tends to protect it.
Performance testing under real workloads is going to give better info
here.

merlin