Re: Proposed patch for operator lookup caching

From: Simon Riggs <simon(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-patches(at)postgreSQL(dot)org
Subject: Re: Proposed patch for operator lookup caching
Date: 2007-11-27 16:38:09
Message-ID: 1196181489.4246.1030.camel@ebony.site
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

On Mon, 2007-11-26 at 21:13 -0500, 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 see 45% speedup also on my previously published tests.

No noticeable difference on the integer test, so looks good.

> 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.

No, thats enough.

> 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.

Well, as long as we fix this, I don't mind how we do it.

The reason for writing the other patch was your requirement for a
minimally invasive patch. If we're willing to lift that requirement then
I'm happy to go with your patch. Personally, I am.

--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Bruce Momjian 2007-11-27 16:48:01 Help with release note items
Previous Message Brendan Jurd 2007-11-27 16:32:27 Re: [GENERAL] Empty arrays with ARRAY[]

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2007-11-27 17:19:51 Re: [GENERAL] Empty arrays with ARRAY[]
Previous Message Brendan Jurd 2007-11-27 16:32:27 Re: [GENERAL] Empty arrays with ARRAY[]