Re: Add ExprState hashing for GROUP BY and hashed SubPlans

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Andrei Lepikhov <lepihov(at)gmail(dot)com>
Cc: PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Add ExprState hashing for GROUP BY and hashed SubPlans
Date: 2024-11-28 07:05:09
Message-ID: CAApHDvr8Zc0ZgzVoCZLdHGOFNhiJeQ6vrUcS9V7N23zMWQb-eA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, 6 Nov 2024 at 09:38, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
> I was performing some final benchmarks on this and found that there's
> a small performance regression for hashed subplans.

> I'm still thinking about what to do about this. In the meantime, I'm
> not going to commit it.

I spent quite a bit more time on this both benchmarking and adjusting
the patch. I've now added some dedicated ExecJust* evaluation
functions for hashing a single Var. This should give something similar
to the JITted performance with optimisation and no inlining for single
columns. The v6-0002 patch does contain some additional ExecJust*
functions to improve the performance of Hash Joins. I'll remove those
and test them independently.

The benchmark results are often very close and often the numbers are
different on both patched and unpatched if I stop and start the server
and prewarm the tables again. For this reason, I ended up running the
benchmarks 50 times each stopping and starting the server between each
run. I added a 10-second sleep as it's quite warm weather here
currently and I did start to get thermal throttling on the Zen4 laptop
without this.

Setup:
create table t1 (a int);
insert into t1 values(1);
create table t2 (a int);
insert into t2 select 1 from generate_series(1,1000000);
vacuum freeze analyze t1,t2;

notin.sql: select a from t2 where a not in (select a from t1);

I made it so the hash table contains only a single record and I probe
that record each time. I did that to ensure the hash table item was
always in L1.

groupby.sql: select a from t2 group by a;

This produces a single group. That was done to eliminate CPU cache
related variations and to try and exaggerate any difference with the
ExprState hashing performance as much as possible.

for i in {1..50}; do pg_ctl stop -D pgdata > /dev/null && pg_ctl start
-D pgdata -l pg.log > /dev/null && psql -c "select
pg_prewarm('t1'),pg_prewarm('t2');" postgres > /dev/null && sleep 10
&& pgbench -n -f notin.sql -T 10 -M prepared postgres | grep tps; done

Please see the attached 6 graphs for the results. Note the left axis
is not scaled to zero. I adjusted the lower end of that axis to be
just below the lowest results value as without that, the lines were
often too close to notice the difference. I also included a dotted
line with the percentage of performance increase the patched version
gave. These are on the righthand vertical axis. For both AMD
machines, I compiled with gcc and clang.

The only slowdown I saw was the NOT IN test on the AMD3990x machine.
It's about 3.6% slower with gcc and 1% slower with clang. All the
other tests show a performance gain. The AMD3990x's GROUP BY test saw
a 13% improvement and the NOT IN test on the Zen4 with gcc averages
almost 33% faster.

I'm pretty happy with this now. The only part I'm not so sure about is
what to pass as the "parent" parameter for ExecBuildHash32FromAttrs()
in nodeSubplan.c. I think the existing call to
ExecBuildGroupingEqual() is passing the wrong PlanState. I believe
this should be passing the PlanState for the SubPlanState.

David

Attachment Content-Type Size
v6-0001-Use-ExprStates-for-hashing-in-GROUP-BY-and-SubPla.patch application/octet-stream 20.3 KB
v6-0002-Add-dedicated-evaluation-functions-for-hashing-Ex.patch application/octet-stream 9.5 KB
image/png 53.2 KB
image/png 39.3 KB
image/png 44.3 KB
image/png 41.3 KB
image/png 38.4 KB
image/png 45.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Corey Huinker 2024-11-28 07:19:30 Re: More CppAsString2() in psql's describe.c
Previous Message Alexander Lakhin 2024-11-28 07:00:00 Re: Announcing Release 18 of the PostgreSQL Buildfarm client