From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Subject: Another modest proposal for reducing CLOBBER_CACHE_ALWAYS runtime
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <575842.1620626589.0@sss.pgh.pa.us>
Date: Mon, 10 May 2021 02:03:58 -0400
Message-ID: <575884.1620626638@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/575884.1620626638%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <575842.1620626589.1@sss.pgh.pa.us>

I noted that, while privileges.sql doesn't stand out in terms of
runtime normally (it's only the fourth slowest test in its
parallel group), it looks absolutely horrid in CLOBBER_CACHE_ALWAYS
testing.  On hyrax's latest run, it takes nearly 9000 seconds longer
than the next-slowest member of its group.  Remembering that the
core regression tests are run thrice in a minimal buildfarm cycle,
this test is single-handedly responsible for over seven hours of the
54 hour total build cycle.

I dug into it and found that the core issue is much like that in
opr_sanity.sql, namely that we're repeating this plpgsql function
$bignum times:

CREATE FUNCTION leak(integer,integer) RETURNS boolean
  AS $$begin return $1 < $2; end$$
  LANGUAGE plpgsql immutable;

(I wonder whether the planner needs to invoke this function
quite so many times during selectivity estimation.  But,
again, improving that seems like a task for some other day.)

Now, as far as I can see, this function definition isn't doing
anything we can't do with an alias for the underlying int4lt
function: the fact that the implementation is in plpgsql
shouldn't matter at all for the purposes of this test.
So I replaced it, as per the attached patch.

On my machine, the time to run privileges.sql under
debug_invalidate_system_caches_always = 1
drops from

real    293m31.054s
to
real    1m47.807s

Yes, really.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="make-leak-a-C-function.patch";
	charset="us-ascii"
Content-ID: <575842.1620626589.2@sss.pgh.pa.us>
Content-Description: make-leak-a-C-function.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/src/test/regress/expected/privileges.out b/src/test/regress/e=
xpected/privileges.out
index 1b4fc16644..83cff902f3 100644
--- a/src/test/regress/expected/privileges.out
+++ b/src/test/regress/expected/privileges.out
@@ -38,6 +38,11 @@ ALTER GROUP regress_priv_group2 ADD USER regress_priv_u=
ser2;	-- duplicate
 NOTICE:  role "regress_priv_user2" is already a member of role "regress_p=
riv_group2"
 ALTER GROUP regress_priv_group2 DROP USER regress_priv_user2;
 GRANT regress_priv_group2 TO regress_priv_user4 WITH ADMIN OPTION;
+-- prepare non-leakproof function for later
+CREATE FUNCTION leak(integer,integer) RETURNS boolean
+  AS 'int4lt'
+  LANGUAGE internal IMMUTABLE STRICT;  -- but deliberately not LEAKPROOF
+ALTER FUNCTION leak(integer,integer) OWNER TO regress_priv_user1;
 -- test owner privileges
 SET SESSION AUTHORIZATION regress_priv_user1;
 SELECT session_user, current_user;
@@ -233,9 +238,6 @@ ALTER TABLE atest12 SET (autovacuum_enabled =3D off);
 SET default_statistics_target =3D 10000;
 VACUUM ANALYZE atest12;
 RESET default_statistics_target;
-CREATE FUNCTION leak(integer,integer) RETURNS boolean
-  AS $$begin return $1 < $2; end$$
-  LANGUAGE plpgsql immutable;
 CREATE OPERATOR <<< (procedure =3D leak, leftarg =3D integer, rightarg =3D=
 integer,
                      restrict =3D scalarltsel);
 -- views with leaky operator
diff --git a/src/test/regress/sql/privileges.sql b/src/test/regress/sql/pr=
ivileges.sql
index 013bc95c74..3d1a1db987 100644
--- a/src/test/regress/sql/privileges.sql
+++ b/src/test/regress/sql/privileges.sql
@@ -45,6 +45,12 @@ ALTER GROUP regress_priv_group2 ADD USER regress_priv_u=
ser2;	-- duplicate
 ALTER GROUP regress_priv_group2 DROP USER regress_priv_user2;
 GRANT regress_priv_group2 TO regress_priv_user4 WITH ADMIN OPTION;
 =

+-- prepare non-leakproof function for later
+CREATE FUNCTION leak(integer,integer) RETURNS boolean
+  AS 'int4lt'
+  LANGUAGE internal IMMUTABLE STRICT;  -- but deliberately not LEAKPROOF
+ALTER FUNCTION leak(integer,integer) OWNER TO regress_priv_user1;
+
 -- test owner privileges
 =

 SET SESSION AUTHORIZATION regress_priv_user1;
@@ -166,9 +172,6 @@ SET default_statistics_target =3D 10000;
 VACUUM ANALYZE atest12;
 RESET default_statistics_target;
 =

-CREATE FUNCTION leak(integer,integer) RETURNS boolean
-  AS $$begin return $1 < $2; end$$
-  LANGUAGE plpgsql immutable;
 CREATE OPERATOR <<< (procedure =3D leak, leftarg =3D integer, rightarg =3D=
 integer,
                      restrict =3D scalarltsel);
 =


------- =_aaaaaaaaaa0--