Re: Getting better results from valgrind leak tracking

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Getting better results from valgrind leak tracking
Date: 2021-03-28 22:48:47
Message-ID: CAApHDvqyQVSdcEOuLgXKak4+MJbgEumV4+2kOO-oc5Uyi93EiQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, 17 Mar 2021 at 15:31, Andres Freund <andres(at)anarazel(dot)de> wrote:
> I'm a bit confused about the precise design of rs_private /
> ParallelBlockTableScanWorkerData, specifically why it's been added to
> TableScanDesc, instead of just adding it to HeapScanDesc? And why is it
> allocated unconditionally, instead of just for parallel scans?

That's a good point. In hindsight, I didn't spend enough effort
questioning that design in the original patch. I see now that the
rs_private field makes very little sense as we can just store what's
private to heapam in HeapScanDescData.

I've done that in the attached. I added the
ParallelBlockTableScanWorkerData as a pointer field in
HeapScanDescData and change it so we only allocate memory for it for
just parallel scans. The field is left as NULL for non-parallel
scans.

I've also added a pfree in heap_endscan() to free the memory when the
pointer is not NULL. I'm hoping that'll fix the valgrind warning, but
I've not run it to check.

David

Attachment Content-Type Size
parallel_chunk_fix.patch text/plain 4.1 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Zi Yi Xu 2021-03-28 22:52:28 Pgsql Google Summer of Code
Previous Message Stephen Frost 2021-03-28 21:20:41 Re: Add docs stub for recovery.conf