RFR: 8267952: async logging supports to dynamically change tags and decorators [v2]

[David Holmes]

On Thu, 10 Jun 2021 07:06:05 GMT, Xin Liu <xliu at openjdk.org> wrote:

>> To support machines without unaligned memory access, C++ defines "alignment requirement" for class and struct. 
>> Not only sizeof(LogDecorators) == sizeof(uint) == 4, C++ can also guarantee that alignof(logDecorators) is 4. 
>> Therefore, it's impossible that to unaligned access for its field _decorators. 
>> 
>> 
>> class LogDecorators {
>>   uint _decorators;
>> };
>> 
>> 
>> Therefore, I think the current code which use normal assignment is atomic. Of course, using Atomic::store manifests it's an atomic store. I will try to use Atomic::store().
>
> Could you take a look at my argument? IMHO, C++ can guarantee the following code is atomic. 
> Alignment shouldn't an issue. 
> 
> 
> void LogOutput::set_decorators(const LogDecorators& decorators) {
>     _decorators = decorators;
> }
> 
> 
> I manage to support the following code. The generated code is exactly same, but it needs some tricks of c++ meta-programming.  Is it worthy?  if you still insist we should use Atomic::store, how about I create a separate JBS issue and submit a patch?  My concern is that Atomic::load/store makes current code messy. 
> 
> 
> void LogOutput::set_decorators(const LogDecorators& decorators) {
>   Atomic::store(&_decorators, decorators);
> }

C++ makes no guarantees about atomicity unless using C++ atomic operations. Yes they probably are atomic but we don't rely on "probably". If a variable is being accessed lock-free and can take part in data-race then please just use Atomic::load and store for simple accesses (assuming they are correct in the context of how the variable can be used concurrently).

-------------

PR: https://git.openjdk.java.net/jdk/pull/4408