Re: [PATCHES] Proposed patch for operator lookup caching - Mailing list pgsql-hackers

Tom Lane wrote:
> Since Simon seems intent on hacking something in there, here is a patch
> that I think is actually sane for improving operator lookup speed.
> This patch caches all lookups, exact or ambiguous (since even the exact
> ones require multiple cache searches in common cases); and behaves sanely
> in the presence of search_path, pg_operator, or pg_cast changes.
> 
> I see about a 45% speedup (2110 vs 1445 tps) on Guillame Smet's test case.
> On straight pgbench --- which has no ambiguous operators, plus it's not
> read-only --- it's hard to measure any consistent speedup, but I can say
> that it's not slower.  Some other test cases would be nice.
> 
> I went through the code that's being bypassed in some detail, to see what
> dependencies were being skipped over.  I think that as long as we assume
> that no *existing* type changes its domain base type, typtype, array
> status, type category, or preferred-type status, we don't need to flush
> the cache on pg_type changes.  This is a good thing since pg_type changes
> frequently (eg, at temp table create or drop).
> 
> The only case that I believe to be unhandled is that the cache doesn't pay
> attention to ALTER TABLE ... INHERIT / NO INHERIT events.  This means it
> is theoretically possible to return the wrong operator if an operator
> takes a complex type as input and the calling situation involves another
> complex type whose inheritance relationship to that one changes.  That's
> sufficiently far out of the normal case that I'm not very worried about it
> (in fact, we probably have bugs in that area even without this patch,
> since for instance cached plans don't respond to such changes either).
> We could plug the hole by forcing a system-wide cache reset during ALTER
> TABLE ... INHERIT / NO INHERIT, if anyone insists.
> 
> I'm not entirely happy about applying a patch like this so late in
> the beta cycle, but I'd much rather do this than than any of the
> less-than-half-baked ideas that have been floated in the discussion
> so far.

Thanks for the patch.  I can see it is clearly of significant size.

I also noted that you found that the case of:
SELECT col FROM tab WHERE text_col = 'ABC';

also took 37% of CPU in January, I think meaning we had this problem in
8.2.

On the one hand we have a pretty significant patch that we might apply. 
It gives us a major speedup (+30%) for a common query type.  I assume
8.3 was slightly slower than 8.2 only because we have a few more
pg_catalog entries in 8.3 than 8.2.  (I am still baffled how a lookup
function could take so much CPU compared to what else is done for a
query.)

We are also talking about catlog changes for 8.3.  Are we comfortable
doing catalog changes between the beta and RC?  I am wondering if the
right plan is to have someone else review your patch, apply it, make the
catalog changes, and release another beta this weekend. Give the beta
one week of testing and go for RC.  That gives us testing of the patch,
and testing of the catalog changes before going to RC1.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://postgres.enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +