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
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[] |
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[] |