Re: Enabling Checksums

On 05.04.2013 23:25, Kevin Grittner wrote:
> Jeff Davis<pgsql@j-davis.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