From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | pgsql-hackers(at)lists(dot)postgresql(dot)org |
Subject: | Reducing opr_sanity test's runtime under CLOBBER_CACHE_ALWAYS |
Date: | 2021-05-08 19:44:57 |
Message-ID: | 292305.1620503097@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
In a nearby thread I bemoaned the fact that the core regression tests
seem to have gotten significantly slower in the last couple of months,
at least with CCA enabled: hyrax reports completing them in 12:52:44
on 18 March, while its most recent run on 1 May took 14:08:18.
Trying to diagnose the cause overall seemed a bit daunting, but
I thought I'd dig into the opr_sanity test in particular, as it
is one of the slower tests under CCA to start with and had also
slowed down noticeably (from 3701581 ms to 4761183 ms, or 28%).
I was able to complete a bisection using just that test, and
got an unexpected result: most of the slowdown appeared at
ab596105b (BRIN minmax-multi indexes). Apparently the additional
time is simply from having to check the additional pg_amop and
pg_amproc entries, which that patch added quite a few of.
I noticed that all of the slowest queries in that test were dependent
on the binary_coercible() plpgsql function that it uses. Now, that
function has always been a rather lame attempt to approximate the
behavior of the parser's IsBinaryCoercible() function, so I've been
thinking for some time that we ought to get rid of it in favor of
actually using IsBinaryCoercible(). I tried that, by adding a
shim function to regress.c, and got a most gratifying result:
on my machine opr_sanity's runtime with
debug_invalidate_system_caches_always = 1 drops from
29m9s to 3m19s. Without CCA the speedup is far less impressive,
360ms to 305ms, but that's still useful. Especially since this
makes the test strictly more accurate.
(I am thinking that this suggests that plpgsql may be hurt more
by cache clobbers than it really needs to be; but doing anything
about that would require some research.)
Anyway, I propose that we ought to sneak this into HEAD, since
it's only touching test code and not anything production-critical.
The patch is a bit more invasive than I would have liked, because
adding the SQL definition of binary_coercible() to create_function_1
(where the other regress.c functions are declared) didn't work:
that runs after opr_sanity, and just moving it up to before
opr_sanity causes the latter to complain about some of the functions
in it. So I ended up splitting the create_function_1 test into
create_function_0 and create_function_1. It's annoying from a
parallelism standpoint that create_function_0 runs by itself, but
the two parallel groups ahead of it are already full. Maybe we
should rebalance that by moving a few of those tests to run in
parallel with create_function_0, but I didn't do that here.
Thoughts?
regards, tom lane
Attachment | Content-Type | Size |
---|---|---|
make-binary_coercible-a-C-function-1.patch | text/x-diff | 16.2 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2021-05-08 19:52:26 | Re: Will Postgres12 installed on a RHEL 6 server continue to function after the server get O/S upgrade to RHEL 7? |
Previous Message | Ahsan Hadi | 2021-05-08 18:51:45 | Re: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns |