On Thu, Oct 9, 2014 at 8:20 AM, Andres Freund <and...@2ndquadrant.com> wrote:
> On 2014-10-09 08:18:18 -0400, Robert Haas wrote:
>> On Thu, Oct 9, 2014 at 5:34 AM, Andres Freund <and...@2ndquadrant.com> wrote:
>> > Interesting - in my local profile AtStart_Inval() is more pronounced
>> > than AfterTriggerBeginQuery(). I've quickly and in a ugly fashion hacked
>> > AtStart_Inval() out of readonly queries ontop of your patch. Together
>> > that yields a ~3.5% performance improvement in my trivial 'SELECT * FROM
>> > tbl WHER pkey = xxx' testcase.
>>
>> Whoa. Now that's clearly significant.
>
> Well, my guess it'll be far less noticeable in less trivial
> workloads. But it does seem worthwile.
>
>> You didn't attach the patch; was that inadvertent, or was it too ugly
>> for that?
>
> Far, far too ugly ;). I just removed the AtStart() call from xact.c and
> sprinkled it around relevant places instead ;)
OK, here's an attempt at a real patch for that. I haven't perf-tested this.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index 5b5d31b..651a5c4 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -1838,7 +1838,6 @@ StartTransaction(void)
* initialize other subsystems for new transaction
*/
AtStart_GUC();
- AtStart_Inval();
AtStart_Cache();
AfterTriggerBeginXact();
@@ -4151,7 +4150,6 @@ StartSubTransaction(void)
*/
AtSubStart_Memory();
AtSubStart_ResourceOwner();
- AtSubStart_Inval();
AtSubStart_Notify();
AfterTriggerBeginSubXact();
diff --git a/src/backend/utils/cache/inval.c b/src/backend/utils/cache/inval.c
index a7a768e..6b6c88e 100644
--- a/src/backend/utils/cache/inval.c
+++ b/src/backend/utils/cache/inval.c
@@ -693,19 +693,32 @@ AcceptInvalidationMessages(void)
}
/*
- * AtStart_Inval
- * Initialize inval lists at start of a main transaction.
+ * PrepareInvalidationState
+ * Initialize inval lists for the current (sub)transaction.
*/
-void
-AtStart_Inval(void)
+static void
+PrepareInvalidationState(void)
{
- Assert(transInvalInfo == NULL);
- transInvalInfo = (TransInvalidationInfo *)
+ TransInvalidationInfo *myInfo;
+
+ if (transInvalInfo != NULL &&
+ transInvalInfo->my_level == GetCurrentTransactionNestLevel())
+ return;
+
+ myInfo = (TransInvalidationInfo *)
MemoryContextAllocZero(TopTransactionContext,
sizeof(TransInvalidationInfo));
- transInvalInfo->my_level = GetCurrentTransactionNestLevel();
- SharedInvalidMessagesArray = NULL;
- numSharedInvalidMessagesArray = 0;
+ myInfo->parent = transInvalInfo;
+ myInfo->my_level = GetCurrentTransactionNestLevel();
+
+ /*
+ * If there's any previous entry, this one should be for a deeper
+ * nesting level.
+ */
+ Assert(transInvalInfo == NULL ||
+ myInfo->my_level > transInvalInfo->my_level);
+
+ transInvalInfo = myInfo;
}
/*
@@ -727,24 +740,6 @@ PostPrepare_Inval(void)
}
/*
- * AtSubStart_Inval
- * Initialize inval lists at start of a subtransaction.
- */
-void
-AtSubStart_Inval(void)
-{
- TransInvalidationInfo *myInfo;
-
- Assert(transInvalInfo != NULL);
- myInfo = (TransInvalidationInfo *)
- MemoryContextAllocZero(TopTransactionContext,
- sizeof(TransInvalidationInfo));
- myInfo->parent = transInvalInfo;
- myInfo->my_level = GetCurrentTransactionNestLevel();
- transInvalInfo = myInfo;
-}
-
-/*
* Collect invalidation messages into SharedInvalidMessagesArray array.
*/
static void
@@ -803,8 +798,16 @@ xactGetCommittedInvalidationMessages(SharedInvalidationMessage **msgs,
{
MemoryContext oldcontext;
+ /* Quick exit if we haven't done anything with invalidation messages. */
+ if (transInvalInfo == NULL)
+ {
+ *RelcacheInitFileInval = false;
+ *msgs = NULL;
+ return 0;
+ }
+
/* Must be at top of stack */
- Assert(transInvalInfo != NULL && transInvalInfo->parent == NULL);
+ Assert(transInvalInfo->my_level == 1 && transInvalInfo->parent == NULL);
/*
* Relcache init file invalidation requires processing both before and
@@ -904,11 +907,15 @@ ProcessCommittedInvalidationMessages(SharedInvalidationMessage *msgs,
void
AtEOXact_Inval(bool isCommit)
{
+ /* Quick exit if no messages */
+ if (transInvalInfo == NULL)
+ return;
+
+ /* Must be at top of stack */
+ Assert(transInvalInfo->my_level == 1 && transInvalInfo->parent == NULL);
+
if (isCommit)
{
- /* Must be at top of stack */
- Assert(transInvalInfo != NULL && transInvalInfo->parent == NULL);
-
/*
* Relcache init file invalidation requires processing both before and
* after we send the SI messages. However, we need not do anything
@@ -926,17 +933,16 @@ AtEOXact_Inval(bool isCommit)
if (transInvalInfo->RelcacheInitFileInval)
RelationCacheInitFilePostInvalidate();
}
- else if (transInvalInfo != NULL)
+ else
{
- /* Must be at top of stack */
- Assert(transInvalInfo->parent == NULL);
-
ProcessInvalidationMessages(&transInvalInfo->PriorCmdInvalidMsgs,
LocalExecuteInvalidationMessage);
}
/* Need not free anything explicitly */
transInvalInfo = NULL;
+ SharedInvalidMessagesArray = NULL;
+ numSharedInvalidMessagesArray = 0;
}
/*
@@ -960,18 +966,38 @@ AtEOXact_Inval(bool isCommit)
void
AtEOSubXact_Inval(bool isCommit)
{
- int my_level = GetCurrentTransactionNestLevel();
+ int my_level;
TransInvalidationInfo *myInfo = transInvalInfo;
- if (isCommit)
+ /* Quick exit if no messages. */
+ if (myInfo == NULL)
+ return;
+
+ /* Also bail out quickly if messages are not for this level. */
+ my_level = GetCurrentTransactionNestLevel();
+ if (myInfo->my_level != my_level)
{
- /* Must be at non-top of stack */
- Assert(myInfo != NULL && myInfo->parent != NULL);
- Assert(myInfo->my_level == my_level);
+ Assert(myInfo->my_level < my_level);
+ return;
+ }
+ if (isCommit)
+ {
/* If CurrentCmdInvalidMsgs still has anything, fix it */
CommandEndInvalidationMessages();
+ /*
+ * We create invalidation stack entries lazily, so the parent might
+ * not have one. Instead of creating one, moving all the data over,
+ * and then freeing our own, we can just adjust the level of our own
+ * entry.
+ */
+ if (myInfo->parent == NULL || myInfo->parent->my_level < my_level - 1)
+ {
+ myInfo->my_level--;
+ return;
+ }
+
/* Pass up my inval messages to parent */
AppendInvalidationMessages(&myInfo->parent->PriorCmdInvalidMsgs,
&myInfo->PriorCmdInvalidMsgs);
@@ -986,11 +1012,8 @@ AtEOSubXact_Inval(bool isCommit)
/* Need not free anything else explicitly */
pfree(myInfo);
}
- else if (myInfo != NULL && myInfo->my_level == my_level)
+ else
{
- /* Must be at non-top of stack */
- Assert(myInfo->parent != NULL);
-
ProcessInvalidationMessages(&myInfo->PriorCmdInvalidMsgs,
LocalExecuteInvalidationMessage);
@@ -1075,6 +1098,12 @@ CacheInvalidateHeapTuple(Relation relation,
return;
/*
+ * If we're not prepared to queue invalidation messages for this
+ * subtransaction level, get ready now.
+ */
+ PrepareInvalidationState();
+
+ /*
* First let the catcache do its thing
*/
tupleRelId = RelationGetRelid(relation);
@@ -1159,6 +1188,8 @@ CacheInvalidateCatalog(Oid catalogId)
{
Oid databaseId;
+ PrepareInvalidationState();
+
if (IsSharedRelation(catalogId))
databaseId = InvalidOid;
else
@@ -1182,6 +1213,8 @@ CacheInvalidateRelcache(Relation relation)
Oid databaseId;
Oid relationId;
+ PrepareInvalidationState();
+
relationId = RelationGetRelid(relation);
if (relation->rd_rel->relisshared)
databaseId = InvalidOid;
@@ -1202,6 +1235,8 @@ CacheInvalidateRelcacheByTuple(HeapTuple classTuple)
Oid databaseId;
Oid relationId;
+ PrepareInvalidationState();
+
relationId = HeapTupleGetOid(classTuple);
if (classtup->relisshared)
databaseId = InvalidOid;
@@ -1221,6 +1256,8 @@ CacheInvalidateRelcacheByRelid(Oid relid)
{
HeapTuple tup;
+ PrepareInvalidationState();
+
tup = SearchSysCache1(RELOID, ObjectIdGetDatum(relid));
if (!HeapTupleIsValid(tup))
elog(ERROR, "cache lookup failed for relation %u", relid);
diff --git a/src/include/utils/inval.h b/src/include/utils/inval.h
index 6156e02..9842b69 100644
--- a/src/include/utils/inval.h
+++ b/src/include/utils/inval.h
@@ -25,10 +25,6 @@ typedef void (*RelcacheCallbackFunction) (Datum arg, Oid relid);
extern void AcceptInvalidationMessages(void);
-extern void AtStart_Inval(void);
-
-extern void AtSubStart_Inval(void);
-
extern void AtEOXact_Inval(bool isCommit);
extern void AtEOSubXact_Inval(bool isCommit);
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
