RFR: JDK-8266503: [UL] Make Decorations safely copy-able and reduce their size [v2]

[Xin Liu]

On Thu, 6 May 2021 19:47:08 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>> src/hotspot/share/logging/logDecorations.cpp line 74:
>> 
>>> 72: }
>>> 73: 
>>> 74: void LogDecorations::print_decoration(LogDecorators::Decorator decorator, outputStream* st) const {
>> 
>> Is that a good idea to do a sanity check here in debug build? 
>> 
>> In my understanding, calling `print_#name##decoration()` for a decorator which was not set in ctor's decorators is undefined.  Currently, your code is safe because you carefully initialize each member variable in initializer list. You can't assume follower programmers do the same. 
>> 
>> If you save the variable `decorators`(a bitmask), you can do sanity check here.  This can be done in debug build. eg. 
>> `assert(_decorators.is_decorator(decorator), "sanity check");`
>
> Hmm, would not the compiler warn you if you leave member initialization out?
> 
> Your idea is not bad, but I usually avoid adding debug only members because you end up with different memory size and layout of data structures between debug and opt builds, which may have strange consequences when hunting bugs.
> 
> Idk. I can do this here, I have no strong feelings either way.

C++ won't warn you if you don't initialize member variables. Unlike Java,  C++ even doesn't zero them for you by default.  

eg. 

#include <stdio.h>
class T{
public:
    int a, b, c;
    T() : b(0) {}
};

int main() {
    T t;
    printf("%d\n", t.a); // random value on stack
    printf("%d\n", t.b); // 0
    printf("%d\n", t.c); // random value on stack
}


It's relatively safe now because all member variables you added to LogDecorations are "scalars".  If we extend an object decorator, printing an uninitialized object may result in undefined behavior.   

I am fine with/without it. can leave it to the future.

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

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