Re: Convert node test compile-time settings into run-time parameters

On 21.05.24 20:48, Andres Freund wrote:
> Where I'd be more concerned about peformance is the added branch in
> READ_LOCATION_FIELD. There are a lot of calls to that, addding runtime
> branches to each, with external function calls inside, is somewhat likely to
> be measurable.

Ok, I have an improved plan.  I'm wrapping all the code related to this 
in #ifdef DEBUG_NODE_TESTS_ENABLED.  This in turn is defined in 
assert-enabled builds, or if you define it explicitly, or if you define 
one of the legacy individual symbols.  That way you get the run-time 
settings in a normal development build, but there is no new run-time 
overhead.  This is the same scheme that we use for debug_discard_caches.

(An argument could be made to enable this code if and only if assertions 
are enabled, since these tests are themselves kind of assertions.  But I 
think having a separate symbol documents the purpose of the various code 
sections better.)

>> Another thought:  Do we really need three separate settings?
> 
> Maybe not three settings, but a single setting, with multiple values, like
> debug_io_direct?

Yeah, good idea.  Let's get some more feedback on this before I code up 
a complicated list parser.

Another approach might be levels.  My testing showed that the overhead 
of the copy_parse_plan_trees and raw_expression_coverage_tests flags is 
hardly noticeable, but write_read_parse_plan_trees has some noticeable 
impact.  So you could do 0=off, 1=only the cheap ones, 2=all tests.

In fact, if we could make "only the cheap ones" the default for 
assert-enabled builds, then most people won't even need to worry about 
this setting: The only way to mess up the write_read_parse_plan_trees is 
if you write custom node support, which is rare.  But the raw expression 
coverage still needs to be maintained by hand, so it's more often 
valuable to have it checked automatically.