Thread: Possible micro-optimization in CacheInvalidateHeapTuple
CacheInvalidateHeapTuple currently does the following tests first; would there be a performance improvement to testing thesystem relation case first? We're almost never in bootstrap mode, so that test is almost always a waste. Is there anyreason 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 incatcaches 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
Jim Nasby <Jim.Nasby@BlueTreble.com> writes:
> CacheInvalidateHeapTuple currently does the following tests first; would there be a performance improvement to
testingthe system relation case first? We're almost never in bootstrap mode, so that test is almost always a waste. Is
thereany 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
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 testingthe system relation case first? We're almost never in bootstrap mode, so that test is almost always a waste. Is thereany 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
Attachment
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 testingthe system relation case first? We're almost never in bootstrap mode, so that test is almost always a waste. Is thereany 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