Re: [HACKERS] 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.na...@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
Re: [HACKERS] Possible micro-optimization in CacheInvalidateHeapTuple
On 10/13/14, 8:28 PM, Tom Lane wrote: Jim Nasby jim.na...@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 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; -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Possible micro-optimization in CacheInvalidateHeapTuple
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
Re: [HACKERS] Possible micro-optimization in CacheInvalidateHeapTuple
Jim Nasby jim.na...@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
