From 11e178d0dc4bc2328ae4759090b3c48b07023fab Mon Sep 17 00:00:00 2001 From: Kevin Grittner Date: Thu, 21 Apr 2016 08:40:08 -0500 Subject: Inline initial comparisons in TestForOldSnapshot() Even with old_snapshot_threshold = -1 (which disables the "snapshot too old" feature), performance regressions were seen at moderate to high concurrency. For example, a one-socket, four-core system running 200 connections at saturation could see up to a 2.3% regression, with larger regressions possible on NUMA machines. By inlining the early (smaller, faster) tests in the TestForOldSnapshot() function, the i7 case dropped to a 0.2% regression, which could easily just be noise, and is clearly an improvement. Further testing will show whether more is needed. --- src/backend/storage/buffer/bufmgr.c | 28 +++++----------------------- 1 file changed, 5 insertions(+), 23 deletions(-) (limited to 'src/backend/storage/buffer/bufmgr.c') diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c index ac1e513560c..c0d5fa74e10 100644 --- a/src/backend/storage/buffer/bufmgr.c +++ b/src/backend/storage/buffer/bufmgr.c @@ -4282,33 +4282,15 @@ IssuePendingWritebacks(WritebackContext *context) /* - * Check whether the given snapshot is too old to have safely read the given - * page from the given table. If so, throw a "snapshot too old" error. + * Implement slower/larger portions of TestForOldSnapshot * - * This test generally needs to be performed after every BufferGetPage() call - * that is executed as part of a scan. It is not needed for calls made for - * modifying the page (for example, to position to the right place to insert a - * new index tuple or for vacuuming). It may also be omitted where calls to - * lower-level functions will have already performed the test. - * - * Note that a NULL snapshot argument is allowed and causes a fast return - * without error; this is to support call sites which can be called from - * either scans or index modification areas. - * - * For best performance, keep the tests that are fastest and/or most likely to - * exclude a page from old snapshot testing near the front. + * Smaller/faster portions are put inline, but the entire set of logic is too + * big for that. */ void -TestForOldSnapshot(Snapshot snapshot, Relation relation, Page page) +TestForOldSnapshot_impl(Snapshot snapshot, Relation relation) { - Assert(relation != NULL); - - if (old_snapshot_threshold >= 0 - && (snapshot) != NULL - && (snapshot)->satisfies == HeapTupleSatisfiesMVCC - && !XLogRecPtrIsInvalid((snapshot)->lsn) - && PageGetLSN(page) > (snapshot)->lsn - && !IsCatalogRelation(relation) + if (!IsCatalogRelation(relation) && !RelationIsAccessibleInLogicalDecoding(relation) && (snapshot)->whenTaken < GetOldSnapshotThresholdTimestamp()) ereport(ERROR, -- cgit v1.2.3