Re: Enabling Checksums

On 05.04.2013 23:25, Kevin Grittner wrote:
> Jeff Davis<pgsql(at)j-davis(dot)com> wrote:
>> Also, the first version doesn't necessarily need to perform well;
>> we can leave optimization as future work.
>
> +1, as long as we don't slow down instances not using the feature,
> and we don't paint ourselves into a corner.

Speaking of which: I did some profiling yesterday of a test case that's
heavy on WAL insertions, without checksums. I saw BufferGetLSNAtomic
consuming 1.57% of the CPU time. That's not much, but it's clearly
additional overhead caused by the checksums patch:

Events: 6K cycles

+ 26,60% postmaster postgres [.] XLogInsert
+ 6,15% postmaster postgres [.] LWLockAcquire
+ 4,74% postmaster postgres [.] LWLockRelease
+ 2,47% postmaster postgres [.] PageAddItem
+ 2,19% postmaster postgres [.] ReadBuffer_common
+ 2,18% postmaster postgres [.] heap_fill_tuple
+ 1,95% postmaster postgres [.] ExecNestLoop
+ 1,89% postmaster postgres [.] ExecModifyTable
+ 1,85% postmaster postgres [.] heap_insert
+ 1,82% postmaster postgres [.] heap_prepare_insert
+ 1,79% postmaster postgres [.] heap_form_tuple
+ 1,76% postmaster postgres [.] RelationGetBufferForTuple
+ 1,75% postmaster libc-2.13.so [.] __memcpy_ssse3
+ 1,73% postmaster postgres [.] PinBuffer
+ 1,67% postmaster postgres [.] hash_any
+ 1,64% postmaster postgres [.] ExecProcNode
+ 1,63% postmaster postgres [.] RelationPutHeapTuple
+ 1,57% postmaster postgres [.] BufferGetLSNAtomic
+ 1,51% postmaster postgres [.] ExecProject
+ 1,42% postmaster postgres [.] hash_search_with_hash_value
+ 1,34% postmaster postgres [.] AllocSetAlloc
+ 1,21% postmaster postgres [.] UnpinBuffer
+ 1,19% postmaster [kernel.kallsyms] [k] copy_user_generic_string
+ 1,13% postmaster postgres [.] MarkBufferDirty
+ 1,07% postmaster postgres [.] ExecScan
+ 1,00% postmaster postgres [.] ExecMaterializeSlot

AFAICS that could be easily avoided by doing a simple PageGetLSN() like
we used to, if checksums are not enabled. In XLogCheckBuffer:

> /*
> * XXX We assume page LSN is first data on *every* page that can be passed
> * to XLogInsert, whether it otherwise has the standard page layout or
> * not. We don't need the buffer header lock for PageGetLSN because we
> * have exclusive lock on the page and/or the relation.
> */
> *lsn = BufferGetLSNAtomic(rdata->buffer);

Also, the second sentence in the above comment is completely bogus now.

- Heikki