Re: On the need for a snapshot in exec_bind_message()

Hi,

On 2018-09-05 12:31:04 -0700, Daniel Wood wrote:
> NOTE:
> 
> In GetSnapshotData because pgxact, is declared volatile, the compiler will not reduce the following two IF tests into
asingle test:
 
> 
> 
>     if (pgxact->vacuumFlags & PROC_IN_LOGICAL_DECODING)
>         continue;
> 
> 
>     if (pgxact->vacuumFlags & PROC_IN_VACUUM)
>         continue;
> 
> 
> You can reduce the code path in the inner loop by coding this as:
> 
>     if (pgxact->vacuumFlags & (PROC_IN_LOGICAL_DECODING|PROC_IN_VACUUM))
>         continue;
> 
> 
> I'm still working on quantifying any gain.  Note it isn't just one L1 cache
> 
> fetch and one conditional branch eliminated.  Due to the update frequency of the pgxact cache line, for single
statementTXN's, there are a certain number of full cache misses, due to invalidation, that occurs when given pgxact is
updatedbetween the first fetch of vacuumFlags and the 2nd fetch.
 

These two obviously could be combined, but I think we should just get
rid of the volatiles. With a small bit of work they shouldn't be
required anymore these days.

Greetings,

Andres Freund