Re: pgbench: option delaying queries till connectionsestablishment?
От | Fabien COELHO |
---|---|
Тема | Re: pgbench: option delaying queries till connectionsestablishment? |
Дата | |
Msg-id | alpine.DEB.2.21.2003070905250.21542@pseudo обсуждение исходный текст |
Ответ на | Re: pgbench: option delaying queries till connections establishment? (Andres Freund <andres@anarazel.de>) |
Ответы |
Re: pgbench: option delaying queries till connectionsestablishment?
|
Список | pgsql-hackers |
Hello Andres, >> I've changed the performance calculations depending on -C or not. Ramp-up >> effects are smoothed. > >> I've merged all time-related stuff (time_t, instr_time, int64) to use a >> unique type (pg_time_usec_t) and set of functions/macros, which simplifies >> the code somehow. > > Hm. I'm not convinced it's a good idea for pgbench to do its own thing > here. Given the unjustifiable heterogeneousness it induces and the simpler code after the move, I think it is much better. Pgbench cloc is smaller after barrier are added (4655 to 4650) thanks to that and a few other code simplifications. Removing all INSTR_TIME_* costly macros is a relief in itself… >> +static int pthread_barrier_init(pthread_barrier_t *barrier, void *unused, int nthreads); > > How about using 'struct unknown_type *unused' instead of "unused"? Haven't done it because I found no other instances in pg, and anyway this code is only used once locally and NULL is passed. >> +static pthread_barrier_t >> + start_barrier, /* all threads are started */ >> + bench_barrier; /* benchmarking ready to start */ > > We don't really need two barriers here. Indeed. Down to one. >> /* print out results */ > > Given that we're changing the output (for the better) of pgbench again, > I wonder if we should add the pgbench version to the benchmark > output. Not sure about it, but done anyway. >> + pg_time_usec_t total_delay, /* benchmarking time */ >> + pg_time_usec_t conn_total_delay, /* is_connect */ >> + pg_time_usec_t conn_elapsed_delay, /* !is_connect */ >> + int64 latency_late) > > I'm not a fan of naming these 'delay'. To me that doesn't sounds like > it's about the time the total benchmark has taken. I have used '_duration', and tried to clarify some field and variable names depending on what data they actually hold. >> + printf("tps = %f (including reconnection times)\n", tps); >> + printf("tps = %f (without initial connection establishing)\n", tps); > > Keeping these separate makes sense to me, they're just so wildly > different. Yep. I've added a comment about that. >> +static inline pg_time_usec_t >> +pg_time_get_usec(void) > > For me the function name sounds like you're getting the usec out of a > pg_time. Not that it's getting a new timestamp. I've used "pg_time_now()". >> +#define PG_TIME_SET_CURRENT_LAZY(t) \ >> + if ((t) == 0) \ >> + (t) = pg_time_get_usec() > > I'd make it an inline function instead of this. Done "pg_time_now_lazy(&now)" I have also simplified the code around thread creation & join because it was a mess: thread 0 was run in the middle of the stat collection loop… I have updated the doc with actual current output, but excluding the version display which would have to be changed between releases. -- Fabien.
Вложения
В списке pgsql-hackers по дате отправления: