Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hwC2n-0004i0-8I for pgsql-hackers@arkaria.postgresql.org; Fri, 09 Aug 2019 21:03:25 +0000 Received: from localhost ([127.0.0.1] helo=malur.postgresql.org) by malur.postgresql.org with esmtp (Exim 4.89) (envelope-from ) id 1hwC2k-0002FL-G3 for pgsql-hackers@arkaria.postgresql.org; Fri, 09 Aug 2019 21:03:22 +0000 Received: from magus.postgresql.org ([2a02:c0:301:0:ffff::29]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hwC2k-0002FE-7W for pgsql-hackers@lists.postgresql.org; Fri, 09 Aug 2019 21:03:22 +0000 Received: from sss.pgh.pa.us ([66.207.139.130]) by magus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hwC2h-0002Ay-Ep for pgsql-hackers@lists.postgresql.org; Fri, 09 Aug 2019 21:03:21 +0000 Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1]) by sss.pgh.pa.us (8.14.4/8.14.4) with ESMTP id x79L3CmN014961; Fri, 9 Aug 2019 17:03:12 -0400 From: Tom Lane To: David Rowley cc: Jesper Pedersen , Andres Freund , Robert Haas , PostgreSQL Hackers Subject: Re: POC: converting Lists into arrays In-reply-to: References: <481.1551390571@sss.pgh.pa.us> <24783.1551568303@sss.pgh.pa.us> <20190303043424.itzy3ge52xrkpmpr@alap3.anarazel.de> <437.1551637744@sss.pgh.pa.us> <12684.1551723095@sss.pgh.pa.us> <20190304190612.vgqqsowzkrh22623@alap3.anarazel.de> <26464.1551734920@sss.pgh.pa.us> <20190304221101.hdg4vj5fo4eewh3b@alap3.anarazel.de> <20190304235402.nod3gbotk2qtd4nh@alap3.anarazel.de> <1131.1551746172@sss.pgh.pa.us> <14626.1558745627@sss.pgh.pa.us> <29297.1558799327@sss.pgh.pa.us> <25178.1562006685@sss.pgh.pa.us> <25258.1562023641@sss.pgh.pa.us> <2305.1562181615@sss.pgh.pa.us> <17220.1565301350@sss.pgh.pa.us> Comments: In-reply-to David Rowley message dated "Fri, 09 Aug 2019 12:52:15 +1200" MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0" Content-ID: <14928.1565384549.0@sss.pgh.pa.us> Date: Fri, 09 Aug 2019 17:03:12 -0400 Message-ID: <14960.1565384592@sss.pgh.pa.us> List-Id: List-Help: List-Subscribe: List-Post: List-Owner: List-Archive: Precedence: bulk ------- =_aaaaaaaaaa0 Content-Type: text/plain; charset="us-ascii" Content-ID: <14928.1565384549.1@sss.pgh.pa.us> David Rowley writes: > On Fri, 9 Aug 2019 at 09:55, Tom Lane wrote: >> I still have hopes for getting rid of es_range_table_array though, >> and will look at that tomorrow or so. > Yes, please. I've measured that to be quite an overhead with large > partitioning setups. However, that was with some additional code which > didn't lock partitions until it was ... well .... too late... as it > turned out. But it seems pretty good to remove code that could be a > future bottleneck if we ever manage to do something else with the > locking of all partitions during UPDATE/DELETE. I poked at this, and attached is a patch, but again I'm not seeing that there's any real performance-based argument for it. So far as I can tell, if we've got a lot of RTEs in an executable plan, the bulk of the startup time is going into lock (re) acquisition in AcquirePlannerLocks, and/or permissions scanning in ExecCheckRTPerms; both of those have to do work for every RTE including ones that run-time pruning drops later on. ExecInitRangeTable just isn't on the radar. If we wanted to try to improve things further, it seems like we'd have to find a way to not lock unreferenced partitions at all, as you suggest above. But combining that with run-time pruning seems like it'd be pretty horrid from a system structural standpoint: if we acquire locks only during execution, what happens if we find we must invalidate the query plan? Anyway, the attached might be worth committing just on cleanliness grounds, to avoid two-sources-of-truth issues in the executor. But it seems like there's no additional performance win here after all ... unless you've got a test case that shows differently? regards, tom lane ------- =_aaaaaaaaaa0 Content-Type: text/x-diff; name="remove-es_range_table_array-1.patch"; charset="us-ascii" Content-ID: <14928.1565384549.2@sss.pgh.pa.us> Content-Description: remove-es_range_table_array-1.patch Content-Transfer-Encoding: quoted-printable diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMa= in.c index dbd7dd9..7f494ab 100644 --- a/src/backend/executor/execMain.c +++ b/src/backend/executor/execMain.c @@ -2790,7 +2790,6 @@ EvalPlanQualStart(EPQState *epqstate, EState *parent= estate, Plan *planTree) estate->es_snapshot =3D parentestate->es_snapshot; estate->es_crosscheck_snapshot =3D parentestate->es_crosscheck_snapshot; estate->es_range_table =3D parentestate->es_range_table; - estate->es_range_table_array =3D parentestate->es_range_table_array; estate->es_range_table_size =3D parentestate->es_range_table_size; estate->es_relations =3D parentestate->es_relations; estate->es_queryEnv =3D parentestate->es_queryEnv; diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execU= tils.c index c1fc0d5..afd9beb 100644 --- a/src/backend/executor/execUtils.c +++ b/src/backend/executor/execUtils.c @@ -113,7 +113,6 @@ CreateExecutorState(void) estate->es_snapshot =3D InvalidSnapshot; /* caller must initialize this = */ estate->es_crosscheck_snapshot =3D InvalidSnapshot; /* no crosscheck */ estate->es_range_table =3D NIL; - estate->es_range_table_array =3D NULL; estate->es_range_table_size =3D 0; estate->es_relations =3D NULL; estate->es_rowmarks =3D NULL; @@ -720,29 +719,17 @@ ExecOpenScanRelation(EState *estate, Index scanrelid= , int eflags) * ExecInitRangeTable * Set up executor's range-table-related data * - * We build an array from the range table list to allow faster lookup by = RTI. - * (The es_range_table field is now somewhat redundant, but we keep it to - * avoid breaking external code unnecessarily.) - * This is also a convenient place to set up the parallel es_relations ar= ray. + * In addition to the range table proper, initialize arrays that are + * indexed by rangetable index. */ void ExecInitRangeTable(EState *estate, List *rangeTable) { - Index rti; - ListCell *lc; - /* Remember the range table List as-is */ estate->es_range_table =3D rangeTable; = - /* Set up the equivalent array representation */ + /* Set size of associated arrays */ estate->es_range_table_size =3D list_length(rangeTable); - estate->es_range_table_array =3D (RangeTblEntry **) - palloc(estate->es_range_table_size * sizeof(RangeTblEntry *)); - rti =3D 0; - foreach(lc, rangeTable) - { - estate->es_range_table_array[rti++] =3D lfirst_node(RangeTblEntry, lc); - } = /* * Allocate an array to store an open Relation corresponding to each @@ -753,8 +740,8 @@ ExecInitRangeTable(EState *estate, List *rangeTable) palloc0(estate->es_range_table_size * sizeof(Relation)); = /* - * es_rowmarks is also parallel to the es_range_table_array, but it's - * allocated only if needed. + * es_rowmarks is also parallel to the es_range_table, but it's allocate= d + * only if needed. */ estate->es_rowmarks =3D NULL; } diff --git a/src/include/executor/executor.h b/src/include/executor/execut= or.h index 1fb28b4..39c8b3b 100644 --- a/src/include/executor/executor.h +++ b/src/include/executor/executor.h @@ -535,8 +535,7 @@ extern void ExecInitRangeTable(EState *estate, List *r= angeTable); static inline RangeTblEntry * exec_rt_fetch(Index rti, EState *estate) { - Assert(rti > 0 && rti <=3D estate->es_range_table_size); - return estate->es_range_table_array[rti - 1]; + return (RangeTblEntry *) list_nth(estate->es_range_table, rti - 1); } = extern Relation ExecGetRangeTableRelation(EState *estate, Index rti); diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 4ec7849..063b490 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -502,7 +502,6 @@ typedef struct EState Snapshot es_snapshot; /* time qual to use */ Snapshot es_crosscheck_snapshot; /* crosscheck time qual for RI */ List *es_range_table; /* List of RangeTblEntry */ - struct RangeTblEntry **es_range_table_array; /* equivalent array */ Index es_range_table_size; /* size of the range table arrays */ Relation *es_relations; /* Array of per-range-table-entry Relation * pointers, or NULL if not yet opened */ ------- =_aaaaaaaaaa0--