Re: pgbench - doCustom cleanup

On 2018-Nov-20, Fabien COELHO wrote:

> Hmm. It is somehow, but the aim of the refactoring is to make *ALL* state
> transitions to happen in doCustom's switch (st->state) and nowhere else,
> which is defeated by creating the separate function.
> 
> Although it improves readability at one level, it does not help figuring out
> what happens to states, which is my primary concern: The idea is that
> reading doCustom is enough to build and check the automaton, which I had to
> do repeatedly while reviewing Marina's patches.

Yeah, there are conflicting goals here.


I didn't quite understand this hunk.  Why does it remove the
is_latencies conditional?  (The preceding comment shown here should be
updated obviously if this change is correct, but I'm not sure it is.)

@@ -3364,42 +3334,34 @@ doCustom(TState *thread, CState *st, StatsData *agg)
 				/*
 				 * command completed: accumulate per-command execution times
 				 * in thread-local data structure, if per-command latencies
 				 * are requested.
 				 */
-				if (is_latencies)
-				{
-					if (INSTR_TIME_IS_ZERO(now))
-						INSTR_TIME_SET_CURRENT(now);
+				INSTR_TIME_SET_CURRENT_LAZY(now);
 
-					/* XXX could use a mutex here, but we choose not to */
-					command = sql_script[st->use_file].commands[st->command];
-					addToSimpleStats(&command->stats,
-									 INSTR_TIME_GET_DOUBLE(now) -
-									 INSTR_TIME_GET_DOUBLE(st->stmt_begin));
-				}
+				/* XXX could use a mutex here, but we choose not to */
+				command = sql_script[st->use_file].commands[st->command];
+				addToSimpleStats(&command->stats,
+								 INSTR_TIME_GET_DOUBLE(now) -
+								 INSTR_TIME_GET_DOUBLE(st->stmt_begin));


-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services