Reducing opr_sanity test's runtime under CLOBBER_CACHE_ALWAYS
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Date: 2021-05-08T19:44:57Z
Lists: pgsql-hackers
Attachments
- make-binary_coercible-a-C-function-1.patch (text/x-diff) patch
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
Commits
-
Replace opr_sanity test's binary_coercible() function with C code.
- 6303a5730914 14.0 landed
-
Clean up/tighten up coercibility checks in opr_sanity regression test.
- e9f42d529f99 12.0 cited