On Tue, Dec 20, 2011 at 4:34 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Noah Misch <n...@leadboat.com> writes:
>> On Mon, Dec 19, 2011 at 11:13:57PM -0500, Tom Lane wrote:
>>> ... but this performance test seems to me to be entirely misguided,
>>> because it's testing a situation that isn't going to occur much in the
>>> field, precisely because the syscache should prevent constant reloads of
>>> the same syscache entry.
>
>>> [ideas for more-realistic tests]
>
>> Granted, but I don't hope to reliably measure a change in a macro-benchmark
>> after seeing a rickety 2% change in a micro-benchmark.
>
> No, I'm not sure about that at all. In particular I think that
> CatalogCacheFlushCatalog is pretty expensive and so the snapshot costs
> could be a larger part of a more-realistic test.
Attached patch makes SnapshotNow into an MVCC snapshot, initialised at
the start of each scan iff SnapshotNow is passed as the scan's
snapshot. It's fairly brief but seems to do the trick.
Assuming that is the right approach, some timings
10,000 functions individually
Create 15.7s 14.3s 14.8s 14.8s
Drop 11.9s 11.7 11.6s 12.0s
10,000 functions in a schema
Create ... same ...
Drop Cascade 2.5s
That seems OK to me.
Any feedback?
--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 5f6ac2e..8c6d6a1 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -1201,6 +1201,8 @@ heap_beginscan_internal(Relation relation, Snapshot snapshot,
*/
scan->rs_pageatatime = IsMVCCSnapshot(snapshot);
+ InitSnapshotNowIfNeeded(snapshot);
+
/*
* For a seqscan in a serializable transaction, acquire a predicate lock
* on the entire relation. This is required not only to lock all the
@@ -1680,6 +1682,8 @@ heap_get_latest_tid(Relation relation,
if (!ItemPointerIsValid(tid))
return;
+ InitSnapshotNowIfNeeded(snapshot);
+
/*
* Since this can be called with user-supplied TID, don't trust the input
* too much. (RelationGetNumberOfBlocks is an expensive check, so we
diff --git a/src/backend/access/index/indexam.c b/src/backend/access/index/indexam.c
index 16ac4e1..4e03cb5 100644
--- a/src/backend/access/index/indexam.c
+++ b/src/backend/access/index/indexam.c
@@ -292,6 +292,8 @@ index_beginscan_internal(Relation indexRelation,
*/
RelationIncrementReferenceCount(indexRelation);
+ InitSnapshotNowIfNeeded(snapshot);
+
/*
* Tell the AM to open a scan.
*/
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index 5aebbd1..61090be 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -206,6 +206,19 @@ GetLatestSnapshot(void)
}
/*
+ * InitSnapshotNowIfNeeded
+ * If passed snapshot is SnapshotNow then refresh it with latest info.
+ */
+void
+InitSnapshotNowIfNeeded(Snapshot snap)
+{
+ if (!IsSnapshotNow(snap))
+ return;
+
+ snap = GetSnapshotData(&SnapshotNowData);
+}
+
+/*
* SnapshotSetCommandId
* Propagate CommandCounterIncrement into the static snapshots, if set
*/
diff --git a/src/backend/utils/time/tqual.c b/src/backend/utils/time/tqual.c
index 31791a7..61edafa 100644
--- a/src/backend/utils/time/tqual.c
+++ b/src/backend/utils/time/tqual.c
@@ -67,7 +67,7 @@
/* Static variables representing various special snapshot semantics */
-SnapshotData SnapshotNowData = {HeapTupleSatisfiesNow};
+SnapshotData SnapshotNowData = {HeapTupleSatisfiesMVCC};
SnapshotData SnapshotSelfData = {HeapTupleSatisfiesSelf};
SnapshotData SnapshotAnyData = {HeapTupleSatisfiesAny};
SnapshotData SnapshotToastData = {HeapTupleSatisfiesToast};
@@ -293,6 +293,7 @@ HeapTupleSatisfiesSelf(HeapTupleHeader tuple, Snapshot snapshot, Buffer buffer)
return false;
}
+#ifdef RECENTLY_DEAD_CODE
/*
* HeapTupleSatisfiesNow
* True iff heap tuple is valid "now".
@@ -474,6 +475,7 @@ HeapTupleSatisfiesNow(HeapTupleHeader tuple, Snapshot snapshot, Buffer buffer)
HeapTupleHeaderGetXmax(tuple));
return false;
}
+#endif
/*
* HeapTupleSatisfiesAny
diff --git a/src/include/utils/snapmgr.h b/src/include/utils/snapmgr.h
index f195981..a30fb9f 100644
--- a/src/include/utils/snapmgr.h
+++ b/src/include/utils/snapmgr.h
@@ -24,6 +24,7 @@ extern TransactionId RecentGlobalXmin;
extern Snapshot GetTransactionSnapshot(void);
extern Snapshot GetLatestSnapshot(void);
+extern void InitSnapshotNowIfNeeded(Snapshot snap);
extern void SnapshotSetCommandId(CommandId curcid);
extern void PushActiveSnapshot(Snapshot snapshot);
diff --git a/src/include/utils/tqual.h b/src/include/utils/tqual.h
index 0f8a7f8..760fc3f 100644
--- a/src/include/utils/tqual.h
+++ b/src/include/utils/tqual.h
@@ -40,6 +40,8 @@ extern PGDLLIMPORT SnapshotData SnapshotToastData;
/* This macro encodes the knowledge of which snapshots are MVCC-safe */
#define IsMVCCSnapshot(snapshot) \
((snapshot)->satisfies == HeapTupleSatisfiesMVCC)
+#define IsSnapshotNow(snapshot) \
+ ((snapshot) == (&SnapshotNowData))
/*
* HeapTupleSatisfiesVisibility
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
