I've always been annoyed by the implementation of
HeapTupleSatisfiesVisibility (in src/include/utils/tqual.h):
the normal case of an MVCC snapshot is the last one handled,
and we can't expand the set of special cases without slowing
it down even more. Also, as noted in tqual.h, the way we
represent special snapshots is a gross violation of the rules
for writing portable C.
I had an idea about how to make this cleaner and faster at
the same time. Make all snapshots be real pointers to
SnapshotData structures (except InvalidSnapshot, which remains
an alias for NULL). Add a struct field that is a pointer to
the satifies function for the snapshot type. Then
HeapTupleSatisfiesVisibility reduces to something like
((*(snapshot)->satisfies) ((tuple)->t_data, snapshot, buffer))
which ought to be faster than it is now. We could also add
an "mvcc" bool field to simplify IsMVCCSnapshot() --- or just
make it test the contents of the satisfies-function field.
The names SnapshotNow etc would become names of suitably-initialized
global variables (really global constants, since they'll never change).
I'm half inclined to get rid of SnapshotDirty as a global variable,
though: it's ugly because HeapTupleSatisfiesDirty writes into it,
creating a non-reentrant API. Since the above API change causes
HeapTupleSatisfiesDirty to get a pointer to the snapshot struct
it's being used with, callers that need this could use a local
snapshot variable to receive the output xmin/xmax.
This would be cleaner than what we have in a couple of other
ways too: in particular it would eliminate the gotcha that
CopySnapshot/FreeSnapshot don't work on special snapshots.
It might also simplify the idea we were kicking around on
-patches of maintaining reference counts for snapshots;
that's not gonna work for special snapshots either, unless
they become real structs.
Thoughts, objections?
regards, tom lane
