Re: ECPG cleanup and fix for clang compile-time problem

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Peter Eisentraut <peter(at)eisentraut(dot)org>
Cc: John Naylor <johncnaylorls(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Andres Freund <andres(at)anarazel(dot)de>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
Subject: Re: ECPG cleanup and fix for clang compile-time problem
Date: 2024-08-15 19:58:35
Message-ID: 609290.1723751915@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Peter Eisentraut <peter(at)eisentraut(dot)org> writes:
> On 15.08.24 02:43, Tom Lane wrote:
>> I've done some more work on this and hope to post an updated patchset
>> tomorrow. Before that though, is there any objection to going ahead
>> with pushing the 0001 patch (pgindent'ing ecpg's lexer and parser
>> files)? It's pretty bulky yet of no intellectual interest, so I'd
>> like to stop carrying it forward.

> The indentation patch looks good to me and it would be good to get it
> out of the way.

Thanks, done. Here's a revised patchset.

0001-0003 are substantially identical to the previous 0002-0004.
Likewise 0005 is basically the same as previous 0006,
and 0009 is identical to the previous 0007.

0004 differs from the previous 0005 in also moving the
cat_str/make_str functions into util.c, because I found that
at least the make_str functions could be useful in pgc.l.

The new stuff is in 0006-0008, and what it basically does is
clean up all remaining memory leakage in ecpg --- or at least,
all that valgrind can find while running ecpg's regression tests.
(I'm not fool enough to think that there might not be some in
unexercised code paths.) It's fairly straightforward attention
to detail in data structure management.

I discovered the need for more effort on memory leakage by
doing some simple performance testing (basically, running ecpg
on a big file made by pasting together lots of copies of some
of the regression test inputs). v3 was slower and consumed more
memory than HEAD :-(. HEAD does already leak quite a bit of
memory, but v3 was worse, mainly because the string tokens
returned by pgc.l weren't being reclaimed. I hadn't really
set out to drive the leakage to zero, but it turned out to not
be that hard, so I did it.

With those fixes, I see v4 running maybe 10% faster than HEAD,
rather than a similar amount slower. I'm content with that
result, and feel that this may now be commit-quality.

regards, tom lane

Attachment Content-Type Size
v4-0001-Clean-up-documentation-of-parse.pl-and-add-more-i.patch text/x-diff 12.4 KB
v4-0002-Major-cleanup-simplification-and-documentation-of.patch text/x-diff 24.0 KB
v4-0003-Re-implement-ecpg-preprocessor-s-string-managemen.patch text/x-diff 109.5 KB
v4-0004-Move-some-functions-into-a-new-file-ecpg-preproc-.patch text/x-diff 10.1 KB
v4-0005-Improve-ecpg-preprocessor-s-memory-management.patch text/x-diff 88.9 KB
v4-0006-Fix-some-memory-leakage-of-data-type-related-stru.patch text/x-diff 8.6 KB
v4-0007-Make-all-string-valued-tokens-returned-by-pgc.l-b.patch text/x-diff 7.1 KB
v4-0008-Clean-up-some-other-assorted-ecpg-memory-leaks.patch text/x-diff 16.1 KB
v4-0009-Remove-ecpg-s-check_rules.pl.patch text/x-diff 6.9 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2024-08-15 20:20:54 Re: Make query cancellation keys longer
Previous Message Peter Geoghegan 2024-08-15 19:22:32 Showing primitive index scan count in EXPLAIN ANALYZE (for skip scan and SAOP scans)