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

[Xin Liu]

On Wed, 9 Jun 2021 07:13:10 GMT, Xin Liu <xliu at openjdk.org> wrote:

>> src/hotspot/share/logging/logConfiguration.cpp line 268:
>> 
>>> 266:   //
>>> 267:   // LogDecorator is a set of decorators represented in a uint. sizeof(uint) is not greater than a machine word,
>>> 268:   // so store of it is atomic on the mainstream processors. I.e. readers see either its older value or new value.
>> 
>> This is not necessarily true - the field also has to have correct alignment to ensure atomic accesses. The convention we are adopting is that all racily accessed fields like _decorators should be declared volatile, and accessed using Atomic::load and Atomic::store.
>
> 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);
}

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

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