Re: benchmarking Flex practices

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: John Naylor <john(dot)naylor(at)2ndquadrant(dot)com>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: benchmarking Flex practices
Date: 2020-01-12 23:57:50
Message-ID: 5891.1578873470@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

John Naylor <john(dot)naylor(at)2ndquadrant(dot)com> writes:
>> I no longer use state variables to track scanner state, and in fact I
>> removed the existing "state_before" variable in ECPG. Instead, I used
>> the Flex builtins yy_push_state(), yy_pop_state(), and yy_top_state().
>> These have been a feature for a long time, it seems, so I think we're
>> okay as far as portability. I think it's cleaner this way, and
>> possibly faster.

Hmm ... after a bit of research I agree that these functions are not
a portability hazard. They are present at least as far back as flex
2.5.33 which is as old as we've got in the buildfarm.

However, I'm less excited about them from a performance standpoint.
The BEGIN() macro expands to (ordinarily)

yyg->yy_start = integer-constant

which is surely pretty cheap. However, yy_push_state is substantially
more expensive than that, not least because the first invocation in
a parse cycle will involve a malloc() or palloc(). Likewise yy_pop_state
is multiple times more expensive than plain BEGIN().

Now, I agree that this is negligible for ECPG's usage, so if
pushing/popping state is helpful there, let's go for it. But I am
not convinced it's negligible for the backend, and I also don't
see that we actually need to track any nested scanner states there.
So I'd rather stick to using BEGIN in the backend. Not sure about
psql.

BTW, while looking through the latest patch it struck me that
"UCONST" is an underspecified and potentially confusing name.
It doesn't indicate what kind of constant we're talking about,
for instance a C programmer could be forgiven for thinking
it means something like "123U". What do you think of "USCONST",
following UIDENT's lead of prefixing U onto whatever the
underlying token type is?

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tomas Vondra 2020-01-13 00:24:15 Re: Using multiple extended statistics for estimates
Previous Message Daniel Gustafsson 2020-01-12 23:40:20 Re: Question regarding heap_multi_insert documentation