Possible micro-optimization in CacheInvalidateHeapTuple

Started by Jim Nasbyabout 11 years ago4 messages

Jim.Nasby@BlueTreble.com

about 11 years ago

CacheInvalidateHeapTuple currently does the following tests first; would there be a performance improvement to testing the system relation case first? We're almost never in bootstrap mode, so that test is almost always a waste. Is there any reason not to switch the two?

/* Do nothing during bootstrap */
if (IsBootstrapProcessingMode())
return;

/*
* We only need to worry about invalidation for tuples that are in system
* relations; user-relation tuples are never in catcaches and can't affect
* the relcache either.
*/
if (!IsSystemRelation(relation))
return;
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane

tgl@sss.pgh.pa.us

about 11 years ago

In reply to: Jim Nasby (#1)

Re: Possible micro-optimization in CacheInvalidateHeapTuple

Jim Nasby <Jim.Nasby@BlueTreble.com> writes:

CacheInvalidateHeapTuple currently does the following tests first; would there be a performance improvement to testing the system relation case first? We're almost never in bootstrap mode, so that test is almost always a waste. Is there any reason not to switch the two?
/* Do nothing during bootstrap */
if (IsBootstrapProcessingMode())
return;

/*
* We only need to worry about invalidation for tuples that are in system
* relations; user-relation tuples are never in catcaches and can't affect
* the relcache either.
*/
if (!IsSystemRelation(relation))
return;

You're assuming that IsSystemRelation() is safe to apply during bootstrap
mode. Even if it is, I don't see the point of messing with this.
IsBootstrapProcessingMode() is a macro expanding to one comparison
instruction.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Jim Nasby

Jim.Nasby@BlueTreble.com

about 11 years ago

In reply to: Tom Lane (#2)

1 attachment(s)

Re: Possible micro-optimization in CacheInvalidateHeapTuple

On 10/13/14, 8:28 PM, Tom Lane wrote:

Jim Nasby <Jim.Nasby@BlueTreble.com> writes:

CacheInvalidateHeapTuple currently does the following tests first; would there be a performance improvement to testing the system relation case first? We're almost never in bootstrap mode, so that test is almost always a waste. Is there any reason not to switch the two?
/* Do nothing during bootstrap */
if (IsBootstrapProcessingMode())
return;

/*
* We only need to worry about invalidation for tuples that are in system
* relations; user-relation tuples are never in catcaches and can't affect
* the relcache either.
*/
if (!IsSystemRelation(relation))
return;

You're assuming that IsSystemRelation() is safe to apply during bootstrap
mode. Even if it is, I don't see the point of messing with this.
IsBootstrapProcessingMode() is a macro expanding to one comparison
instruction.

Comment patch to that effect attached.
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.com

Attachments:

patchtext/plain; charset=UTF-8; name=patch; x-mac-creator=0; x-mac-type=0Download

diff --git a/src/backend/utils/cache/inval.c b/src/backend/utils/cache/inval.c
index a7a768e..545ccc5 100644
--- a/src/backend/utils/cache/inval.c
+++ b/src/backend/utils/cache/inval.c
@@ -1055,7 +1055,11 @@ CacheInvalidateHeapTuple(Relation relation,
 	Oid			databaseId;
 	Oid			relationId;
 
-	/* Do nothing during bootstrap */
+	/*
+     * Do nothing during bootstrap. It may seem silly to check this first since
+     * it will almost always be false, but it's not safe to assume that later
+     * checks can be done safely while in bootstrap.
+     */
 	if (IsBootstrapProcessingMode())
 		return;

Andres Freund

andres@2ndquadrant.com

about 11 years ago

In reply to: Jim Nasby (#3)

Re: Possible micro-optimization in CacheInvalidateHeapTuple

On 2014-10-21 19:06:41 -0500, Jim Nasby wrote:

On 10/13/14, 8:28 PM, Tom Lane wrote:

Jim Nasby <Jim.Nasby@BlueTreble.com> writes:

CacheInvalidateHeapTuple currently does the following tests first; would there be a performance improvement to testing the system relation case first? We're almost never in bootstrap mode, so that test is almost always a waste. Is there any reason not to switch the two?
/* Do nothing during bootstrap */
if (IsBootstrapProcessingMode())
return;

/*
* We only need to worry about invalidation for tuples that are in system
* relations; user-relation tuples are never in catcaches and can't affect
* the relcache either.
*/
if (!IsSystemRelation(relation))
return;

You're assuming that IsSystemRelation() is safe to apply during bootstrap
mode. Even if it is, I don't see the point of messing with this.
IsBootstrapProcessingMode() is a macro expanding to one comparison
instruction.

Comment patch to that effect attached.

That doesn't seem worth the effort of apply a patch and tracking in the
CF. Marked as returned with feedback.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers