Re: Converting tab-complete.c's else-if chain to a switch - Mailing list pgsql-hackers
From | Andres Freund |
---|---|
Subject | Re: Converting tab-complete.c's else-if chain to a switch |
Date | |
Msg-id | 20240715174016.ukm6k5shwszmudb7@awork3.anarazel.de Whole thread Raw |
In response to | Re: Converting tab-complete.c's else-if chain to a switch (Tom Lane <tgl@sss.pgh.pa.us>) |
Responses |
Re: Converting tab-complete.c's else-if chain to a switch
|
List | pgsql-hackers |
Hi, On 2024-07-13 13:16:14 -0400, Tom Lane wrote: > I wrote: > > Per discussion elsewhere [1], I've been looking at $SUBJECT. > > Here's a finished patchset to perform this change. Awesome! > With gcc 8.5.0 I see the time drop from about 3 seconds to about 0.7. The > win is less noticeable with clang version 15.0 on a Mac M1: from about 0.7s > to 0.45s. Nice! With gcc 15 I see: before after -O0 1.2s 0.6s -O2 10.5s 2.6s -O3 10.8s 2.8s Quite a nice win. > Of course the main point is the hope that it will work at all with MSVC, but > I'm not in a position to test that. It does work with just your patches applied - very nice. The only obvious thing that doesn't is that ctrl-c without a running query terminates psql - interrupting a running query works without terminating psql, which is a bit weird. But that seems independent of this series. > Subject: [PATCH v1 3/5] Install preprocessor infrastructure for > tab-complete.c. Ah - I was wondering how the above would actually work when I read your intro :) > +tab-complete.c: gen_tabcomplete.pl tab-complete.in.c > + $(PERL) $^ --outfile $@ > + When I first built this with make, I got this error: make: *** No rule to make target '/home/andres/src/postgresql/src/bin/psql/tab-complete.c', needed by 'tab-complete.o'. Stop. but that's just a a consequence of the make dependency handling... Removing the .deps directory fixed it. > +# The input is a C file that contains some case labels that are not > +# constants, but look like function calls, for example: > +# case Matches("A", "B"): > +# The function name can be any one of Matches, HeadMatches, TailMatches, > +# MatchesCS, HeadMatchesCS, or TailMatchesCS. The argument(s) must be > +# string literals or macros that expand to string literals or NULL. Hm, the fact that we are continuing to use the same macros in the switch makes it a bit painful to edit the .in.c in an editor with compiler-warnings integration - I see a lot of reported errors ("Expression is not an integer constant expression") due to case statements not being something that the normal C switch can handle. Perhaps we could use a distinct macro for use in the switch, which generates valid (but nonsensical) code, so we avoid warnings? > +# These lines are replaced by "case N:" where N is a unique number > +# for each such case label. (For convenience, we use the line number > +# of the case label, although other values would work just as well.) Hm, using the line number seems to make it a bit harder to compare the output of the script after making changes to the input. Not the end of the world, but ... > +tabcomplete = custom_target('tabcomplete', > + input: 'tab-complete.in.c', > + output: 'tab-complete.c', > + command: [ > + perl, files('gen_tabcomplete.pl'), files('tab-complete.in.c'), > + '--outfile', '@OUTPUT@', > + '@INPUT@', > + ], > + install: true, > + install_dir: dir_include_server / 'utils', > +) I don't think we want to install tab-complete.c? Greetings, Andres Freund
pgsql-hackers by date: