RFR: 8282721: HotSpot Style Guide should allow considered use of C++ thread_local [v2]

[John R Rose]

On Tue, 8 Mar 2022 16:09:52 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> David Holmes has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Feedback from kbarrett and jrose.
>
> doc/hotspot-style.md line 468:
> 
>> 466: (operator new and related functions). Typically, uses of the global
>> 467: operator new are inadvertent and therefore often associated with memory
>> 468: leaks. Use of these functions by HotSpot code is disabled for some platforms.
> 
> I don't agree with the new sentence about uses of global operator new.  "Normal" C++ use of global operator new is no more associated with memory leaks than are the other allocations we do in HotSpot. The rationale for disallowing use of global operator new in HotSpot code (as I understand it) is that we want all of our heap allocations to be trackable via NMT. Any uses of global operator new would bypass that.

First, it's not exactly a new sentence, just one moved from elsewhere in our code base (from a file that was deleted in the companion PR to this one).

Second, it is true; we have seen problems in the (distant) past of exactly the form claimed.  The problem is that HotSpot is an irregular user of C++, including via assembly code and tortuous stack frame manipulation (deopt handlers etc.).  It's easy to accidentally emit a use of of global `op new` through ten layers of C++ header file, and in HotSpot it's also easy to break the careful matching of constructors to destructors that C++ relies on.  The result is a storage leak.

Kim, I could see you thinking, also, that this sort of observation doesn't belong in a style guide, and a lot of these nuggets might tend to bloat which obscures the useful parts of the style guide.  (An over-long guide is not a useful guide after all.)  You might suggest where this rationale information goes, if not here.  But I think it fits well enough here.  And if it isn't inserted here, or some other new place, it will be lost because of David's file deletion in the other PR related to this one.  I don't want it to get lost.

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

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