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

Thomas Stuefe stuefe at openjdk.java.net
Thu May 6 19:50:02 UTC 2021


On Thu, 6 May 2021 19:23:22 GMT, Xin Liu <xliu at openjdk.org> wrote:

>> Thomas Stuefe has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains six additional commits since the last revision:
>> 
>>  - Merge
>>  - Remove harfbuzz build fix
>>  - Volker feedback
>>  - start
>>  - cherry-picked JDK-8266536
>>  - harfbuzz-buildfix
>
> 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.

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

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


More information about the hotspot-runtime-dev mailing list