Re: [HACKERS] pgbench tap tests & minor fixes - Mailing list pgsql-hackers

From Fabien COELHO
Subject Re: [HACKERS] pgbench tap tests & minor fixes
Date
Msg-id alpine.DEB.2.20.1705091641150.29373@lancre
Whole thread Raw
In response to Re: [HACKERS] pgbench tap tests & minor fixes  (Nikolay Shaplov <dhyan@nataraj.su>)
Responses Re: [HACKERS] pgbench tap tests & minor fixes  (Nikolay Shaplov <dhyan@nataraj.su>)
List pgsql-hackers
Hello Nikolay,

> Meanwhile I have another question. I'd like to understand why it have been
> done this way.
>
> Why do you place st->cnt++; inside processXactStats function?

The idea is that it must be counted once. There is a "detailed" statistic 
collection fonction which is called when needed (that is the if with 
all the different options which imply better statistics), else we just do 
the counting part and nothing else.

> [...] This code would mislead me to conclusion that st->cnt is 
> incremented only on "else" branch. But actually it is incremented 
> anyway, but you should carefully read processXactStats function to 
> understand it.

Yep. The increments are just a poor's man (few cycle) stat collection, 
when no detailed stats are needed.

> The second time processXactStats is called inside the while loop we've
> discussed in previous letters. There is no alternative st->cnt++ here, if we
> move st->cnt++ out of processXactStats to the body of the loop, we will be
> able to write detailed comment, explaining why exactly we increment it, so I
> would be the last person who was confused by this part of the code.

Hmmm.

> We will have to move thread->stats.cnt++; then. If we decided to move st-
>> cnt++;. There would not be great harm, as  thread->stats.cnt++; is also done
> in the code outside of processXactStats.
>
> So it is just my idea to made the code better. May be there is good reason for
> keeping it inside processXactStats, I just can't see. What do you think about
> it?

I understand.

I wanted to have *one* single functions where stats are collected, the 
situation before was not too clear about where & when the stats where 
collected. There was some pain to avoid useless cycles, so the function 
was skipped in some cases.

I've modified the code so that the processXactStats function is always 
called, but just does the counting and skip everything else when detailed 
stats are not needed. I think it is a good compromise between simplicity 
and performance. See attached. Hopefully it will be clearer this way.

-- 
Fabien.
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Attachment

pgsql-hackers by date:

Previous
From: Peter Eisentraut
Date:
Subject: Re: [HACKERS] SUBSCRIPTIONS and pg_upgrade
Next
From: Pavel Stehule
Date:
Subject: [HACKERS] export import bytea from psql