From: | David Rowley <dgrowleyml(at)gmail(dot)com> |
---|---|
To: | PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, Magnus Hagander <magnus(at)hagander(dot)net> |
Subject: | Re: Making JIT more granular |
Date: | 2022-04-26 05:24:02 |
Message-ID: | CAApHDvoq5VhV=2euyjgBN2bC8Bds9Dtr0bG7R=reeefJWKJRXA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
(This is an old thread. See [1] if you're missing the original email.)
On Tue, 4 Aug 2020 at 14:01, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> At the moment JIT compilation, if enabled, is applied to all
> expressions in the entire plan. This can sometimes be a problem as
> some expressions may be evaluated lots and warrant being JITted, but
> others may only be evaluated just a few times, or even not at all.
>
> This problem tends to become large when table partitioning is involved
> as the number of expressions in the plan grows with each partition
> present in the plan. Some partitions may have many rows and it can be
> useful to JIT expression, but others may have few rows or even no
> rows, in which case JIT is a waste of effort.
This patch recently came up again in [2], where Magnus proposed we add
a new GUC [3] to warn users when JIT compilation takes longer than the
specified fraction of execution time. Over there I mentioned that I
think it might be better to have a go at making the JIT costing better
so that it's more aligned to the amount of JITing work there is to do
rather than the total cost of the plan without any consideration about
how much there is to JIT compile.
In [4], Andres reminded me that I need to account for the number of
times a given plan is (re)scanned rather than just the total_cost of
the Plan node. There followed some discussion about how that might be
done.
I've loosely implemented this in the attached patch. In order to get
the information about the expected number of "loops" a given Plan node
will be subject to, I've modified create_plan() so that it passes this
value down recursively while creating the plan. Nodes such as Nested
Loop multiply the "est_calls" by the number of outer rows. For nodes
such as Material, I've made the estimated calls = 1.0. Memoize must
take into account the expected cache hit ratio, which I've had to
record as part of MemoizePath so that create_plan knows about that.
Altogether, this is a fair bit of churn for createplan.c, and it's
still only part of the way there. When planning subplans, we do
create_plan() right away and since we plan subplans before the outer
plans, we've no idea how many times the subplan will be rescanned. So
to make this work fully I think we'd need to modify the planner so
that we delay the create_plan() for subplans until sometime after
we've planned the outer query.
The reason that I'm posting about this now is mostly because I did say
I'd come back to this patch for v16 and I'm also feeling bad that I
-1'd Magnus' patch, which likely resulted in making zero forward
progress in improving JIT and it's costing situation for v15.
The reason I've not completed this patch to fix the deficiencies
regarding subplans is that that's quite a bit of work and I don't
really want to do that right now. We might decide that JIT costing
should work in a completely different way that does not require
estimating how many times a plan node will be rescanned. I think
there's enough patch here to allow us to test this and then decide if
it's any good or not.
There's also maybe some controversy in the patch. I ended up modifying
EXPLAIN so that it shows loops=N as part of the estimated costs. I
understand there's likely to be fallout from doing that as there are
various tools around that this would likely break. I added that for a
couple of reasons; 1) I think it would be tricky to figure out why JIT
was or was not enabled without showing that in EXPLAIN, and; 2) I
needed to display it somewhere for my testing so I could figure out if
I'd done something wrong when calculating the value during
create_plan().
This basically looks like:
postgres=# explain select * from h, h h1, h h2;
QUERY PLAN
--------------------------------------------------------------------------
Nested Loop (cost=0.00..12512550.00 rows=1000000000 width=12)
-> Nested Loop (cost=0.00..12532.50 rows=1000000 width=8)
-> Seq Scan on h (cost=0.00..15.00 rows=1000 width=4)
-> Materialize (cost=0.00..20.00 rows=1000 width=4 loops=1000)
-> Seq Scan on h h1 (cost=0.00..15.00 rows=1000 width=4)
-> Materialize (cost=0.00..20.00 rows=1000 width=4 loops=1000000)
-> Seq Scan on h h2 (cost=0.00..15.00 rows=1000 width=4)
(7 rows)
Just the same as EXPLAIN ANALYZE, I've coded loops= to only show when
there's more than 1 loop. You can also see that the node below
Materialize is not expected to be scanned multiple times. Technically
it could when a parameter changes, but right now it seems more trouble
than it's worth to go to the trouble of estimating that during
create_plan(). There's also some variation from the expected loops and
the actual regarding parallel workers. In the estimate, this is just
the number of times an average worker is expected to invoke the plan,
whereas the actual "loops" is the sum of each worker's invocations.
The other slight controversy that I can see in the patch is
repurposing the JIT cost GUCs and giving them a completely different
meaning than they had previously. I've left them as-is for now as I
didn't think renaming GUCs would ease the pain that DBAs would have to
endure as a result of this change.
Does anyone have any thoughts about this JIT costing? Is this an
improvement? Is there a better way?
David
[1] https://www.postgresql.org/message-id/CAApHDvpQJqLrNOSi8P1JLM8YE2C+ksKFpSdZg=q6sTbtQ-v=aw@mail.gmail.com
[2] https://www.postgresql.org/message-id/CAApHDvrEoQ5p61NjDCKVgEWaH0qm1KprYw2-7m8-6ZGGJ8A2Dw%40mail.gmail.com
[3] https://www.postgresql.org/message-id/CABUevExR_9ZmkYj-aBvDreDKUinWLBBpORcmTbuPdNb5vGOLtA%40mail.gmail.com
[4] https://www.postgresql.org/message-id/20220329231641.ai3qrzpdo2vqvwix%40alap3.anarazel.de
Attachment | Content-Type | Size |
---|---|---|
granular_jit_v2.patch | text/plain | 97.0 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2022-04-26 05:25:14 | Re: pgsql: Add contrib/pg_walinspect. |
Previous Message | Michael Paquier | 2022-04-26 05:17:11 | Re: CLUSTER on partitioned index |