Thread

Minor regexp hacking: code coverage, moveins() and friends

Tom Lane <tgl@sss.pgh.pa.us> — 2021-08-15T21:47:43Z

While trying to improve the code-coverage report for backend/regex/,
I found that there are portions of copyins() and copyouts() that are
just plain unreachable, because those functions are over-engineered.
In point of fact, the only uses of copyins() and copyouts() are in
places where the target state is brand new and cannot have any
pre-existing in-arcs (resp. out-arcs).  This means that all the
trouble we're going to to de-duplicate the copied arcs is entirely
wasted; we could just copy the source arcs without extra checking.

A fairly significant fraction, though by no means all, of the calls
of moveins() and moveouts() are likewise working with new target
states, and so don't really need to do any de-duplication.

Hence I propose 0001 attached, which creates simplified functions
copyinstonew() and so on, for use when the target state is known not
to have any existing arcs.  I'd thought that this might show a useful
improvement in regexp compilation speed, but it's pretty hard to
measure any noticeable change on typical regexps such as Jacobson's
web corpus.  (I do see maybe a 1% improvement on that, but that's
below the noise threshold so I don't take it too seriously.)  It is
possible to demonstrate noticeable improvement on handpicked regexes,
for example on HEAD:

regression=# SELECT regexp_matches('foo', 'abcdefghijklmnopq((\y|.?)+)+','');
 regexp_matches 
----------------
(0 rows)

Time: 6.297 ms

versus with patch:

regression=# SELECT regexp_matches('foo', 'abcdefghijklmnopq((\y|.?)+)+','');
 regexp_matches 
----------------
(0 rows)

Time: 5.506 ms

So this isn't entirely a waste of time, but it is marginal.  Improving
the code-coverage numbers is probably a better argument.  (0001 also
adds some test cases that exercise nearly everything that's reachable
without OOM conditions or cancels in regc_nfa.c.)

0002 below is some additional test cases to improve code coverage in
regc_locale.c and regc_pg_locale.c.  (regc_pg_locale.c is still not
great, but that's mostly because much of the code is only reachable
for particular choices of database encoding, so any one coverage
run hits just some of it.)

Barring objections, I plan to push this in a day or two; I don't
think it needs much review.

			regards, tom lane