Re: Free list same_input_transnos in preprocess_aggref

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Zhang Mingli <zmlpostgres(at)gmail(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Free list same_input_transnos in preprocess_aggref
Date: 2022-11-06 20:12:52
Message-ID: 3048711.1667765572@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Zhang Mingli <zmlpostgres(at)gmail(dot)com> writes:
> Correction: SaveBytes = Sum results of accumulate_list_size: 24(4+4+8+8),

What I did was to stick in

elog(LOG, "leaking list of length %d", list_length(same_input_transnos));

at the end of preprocess_aggref. What I see on your five-aggregate
example is

2022-11-06 14:59:25.666 EST [3046253] LOG: leaking list of length 0
2022-11-06 14:59:25.666 EST [3046253] STATEMENT: explain select max(id), min(id), sum(id), count(id), avg(id) from t1;
2022-11-06 14:59:25.666 EST [3046253] LOG: leaking list of length 1
2022-11-06 14:59:25.666 EST [3046253] STATEMENT: explain select max(id), min(id), sum(id), count(id), avg(id) from t1;
2022-11-06 14:59:25.666 EST [3046253] LOG: leaking list of length 0
2022-11-06 14:59:25.666 EST [3046253] STATEMENT: explain select max(id), min(id), sum(id), count(id), avg(id) from t1;
2022-11-06 14:59:25.666 EST [3046253] LOG: leaking list of length 1
2022-11-06 14:59:25.666 EST [3046253] STATEMENT: explain select max(id), min(id), sum(id), count(id), avg(id) from t1;
2022-11-06 14:59:25.666 EST [3046253] LOG: leaking list of length 0
2022-11-06 14:59:25.666 EST [3046253] STATEMENT: explain select max(id), min(id), sum(id), count(id), avg(id) from t1;

The NIL lists are of course occupying no storage. The two one-element
lists are absolutely, completely negligible in the context of planning
any nontrivial statement. Even the aggtransinfos list that is the
primary output of preprocess_aggref will dwarf that; and we leak
similarly small data structures in probably many hundred places in
the planner.

I went a bit further and ran the core regression tests, then aggregated
the results:

$ grep 'leaking list' postmaster.log | sed 's/.*] //' | sort | uniq -c
4516 LOG: leaking list of length 0
95 LOG: leaking list of length 1
15 LOG: leaking list of length 2

You can quibble of course about how representative the regression tests
are, but there's sure no evidence at all here that we'd be saving
anything measurable.

If anything, I'd be inclined to get rid of the

list_free(*same_input_transnos);

in find_compatible_agg, because it seems like a waste of code on
the same grounds. Instrumenting that in the same way, I find
that it's not reached at all in your example, while the
regression tests give

49 LOG: freeing list of length 0
2 LOG: freeing list of length 1

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2022-11-06 20:40:30 Re: Allow single table VACUUM in transaction block
Previous Message Tom Lane 2022-11-06 19:14:28 Re: Allow single table VACUUM in transaction block