8282721: HotSpot Style Guide should allow considered use of C++ thread_local

Reviewed-by: kbarrett, jrose, dcubed, stuefe, mdoerr, kvn
This commit is contained in:
David Holmes 2022-03-22 01:12:29 +00:00
parent f8878cb0cc
commit f3dc0c88ea
2 changed files with 24 additions and 16 deletions

View File

@ -210,7 +210,7 @@ while ( test_foo(args...) ) { // No, excess spaces around control</code></pre></
<p>Rationale: Other than to implement exceptions (which HotSpot doesn't use), most potential uses of <a href="https://en.wikipedia.org/wiki/Run-time_type_information" title="Runtime Type Information">RTTI</a> are better done via virtual functions. Some of the remainder can be replaced by bespoke mechanisms. The cost of the additional runtime data structures needed to support <a href="https://en.wikipedia.org/wiki/Run-time_type_information" title="Runtime Type Information">RTTI</a> are deemed not worthwhile, given the alternatives.</p>
<h3 id="memory-allocation">Memory Allocation</h3>
<p>Do not use the standard global allocation and deallocation functions (operator new and related functions). Use of these functions by HotSpot code is disabled for some platforms.</p>
<p>Rationale: HotSpot often uses &quot;resource&quot; or &quot;arena&quot; allocation. Even where heap allocation is used, the standard global functions are avoided in favor of wrappers around malloc and free that support the VM's Native Memory Tracking (NMT) feature.</p>
<p>Rationale: HotSpot often uses &quot;resource&quot; or &quot;arena&quot; allocation. Even where heap allocation is used, the standard global functions are avoided in favor of wrappers around malloc and free that support the VM's Native Memory Tracking (NMT) feature. Typically, uses of the global operator new are inadvertent and therefore often associated with memory leaks.</p>
<p>Native memory allocation failures are often treated as non-recoverable. The place where &quot;out of memory&quot; is (first) detected may be an innocent bystander, unrelated to the actual culprit.</p>
<h3 id="class-inheritance">Class Inheritance</h3>
<p>Use public single inheritance.</p>
@ -270,8 +270,8 @@ while ( test_foo(args...) ) { // No, excess spaces around control</code></pre></
<p>The underlying type of a <em>scoped-enum</em> should also be specified explicitly if conversions may be applied to values of that type.</p>
<p>Due to bugs in certain (very old) compilers, there is widespread use of enums and avoidance of in-class initialization of static integral constant members. Compilers having such bugs are no longer supported. Except where an enum is semantically appropriate, new code should use integral constants.</p>
<h3 id="thread_local">thread_local</h3>
<p>Do not use <code>thread_local</code> (<a href="http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2659.htm">n2659</a>); instead, use the HotSpot macro <code>THREAD_LOCAL</code>. The initializer must be a constant expression.</p>
<p>As was discussed in the review for <a href="https://mail.openjdk.java.net/pipermail/hotspot-dev/2019-September/039487.html">JDK-8230877</a>, <code>thread_local</code> allows dynamic initialization and destruction semantics. However, that support requires a run-time penalty for references to non-function-local <code>thread_local</code> variables defined in a different translation unit, even if they don't need dynamic initialization. Dynamic initialization and destruction of namespace-scoped thread local variables also has the same ordering problems as for ordinary namespace-scoped variables.</p>
<p>Avoid use of <code>thread_local</code> (<a href="http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2659.htm">n2659</a>); and instead, use the HotSpot macro <code>THREAD_LOCAL</code>, for which the initializer must be a constant expression. When <code>thread_local</code> must be used, use the Hotspot macro <code>APPROVED_CPP_THREAD_LOCAL</code> to indicate that the use has been given appropriate consideration.</p>
<p>As was discussed in the review for <a href="https://mail.openjdk.java.net/pipermail/hotspot-dev/2019-September/039487.html">JDK-8230877</a>, <code>thread_local</code> allows dynamic initialization and destruction semantics. However, that support requires a run-time penalty for references to non-function-local <code>thread_local</code> variables defined in a different translation unit, even if they don't need dynamic initialization. Dynamic initialization and destruction of non-local <code>thread_local</code> variables also has the same ordering problems as for ordinary non-local variables. So we avoid use of <code>thread_local</code> in general, limiting its use to only those cases where dynamic initialization or destruction are essential. See <a href="https://bugs.openjdk.java.net/browse/JDK-8282469">JDK-8282469</a> for further discussion.</p>
<h3 id="nullptr">nullptr</h3>
<p>Prefer <code>nullptr</code> (<a href="http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2431.pdf">n2431</a>) to <code>NULL</code>. Don't use (constexpr or literal) 0 for pointers.</p>
<p>For historical reasons there are widespread uses of both <code>NULL</code> and of integer 0 as a pointer value.</p>

View File

@ -471,7 +471,9 @@ code is disabled for some platforms.
Rationale: HotSpot often uses "resource" or "arena" allocation. Even
where heap allocation is used, the standard global functions are
avoided in favor of wrappers around malloc and free that support the
VM's Native Memory Tracking (NMT) feature.
VM's Native Memory Tracking (NMT) feature. Typically, uses of the global
operator new are inadvertent and therefore often associated with memory
leaks.
Native memory allocation failures are often treated as non-recoverable.
The place where "out of memory" is (first) detected may be an innocent
@ -631,7 +633,7 @@ Here are a few closely related example bugs:<br>
### enum
Where appropriate, _scoped-enums_ should be used.
([n2347](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2347.pdf))
([n2347](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2347.pdf))
Use of _unscoped-enums_ is permitted, though ordinary constants may be
preferable when the automatic initializer feature isn't used.
@ -651,10 +653,12 @@ integral constants.
### thread_local
Do not use `thread_local`
Avoid use of `thread_local`
([n2659](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2659.htm));
instead, use the HotSpot macro `THREAD_LOCAL`. The initializer must
be a constant expression.
and instead, use the HotSpot macro `THREAD_LOCAL`, for which the initializer must
be a constant expression. When `thread_local` must be used, use the Hotspot macro
`APPROVED_CPP_THREAD_LOCAL` to indicate that the use has been given appropriate
consideration.
As was discussed in the review for
[JDK-8230877](https://mail.openjdk.java.net/pipermail/hotspot-dev/2019-September/039487.html),
@ -663,14 +667,18 @@ semantics. However, that support requires a run-time penalty for
references to non-function-local `thread_local` variables defined in a
different translation unit, even if they don't need dynamic
initialization. Dynamic initialization and destruction of
namespace-scoped thread local variables also has the same ordering
problems as for ordinary namespace-scoped variables.
non-local `thread_local` variables also has the same ordering
problems as for ordinary non-local variables. So we avoid use of
`thread_local` in general, limiting its use to only those cases where dynamic
initialization or destruction are essential. See
[JDK-8282469](https://bugs.openjdk.java.net/browse/JDK-8282469)
for further discussion.
### nullptr
Prefer `nullptr`
([n2431](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2431.pdf))
to `NULL`. Don't use (constexpr or literal) 0 for pointers.
to `NULL`. Don't use (constexpr or literal) 0 for pointers.
For historical reasons there are widespread uses of both `NULL` and of
integer 0 as a pointer value.
@ -939,7 +947,7 @@ References:
* Generalized lambda capture (init-capture) ([N3648])
* Generic (polymorphic) lambda expressions ([N3649])
[n2657]: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2657.htm
[n2657]: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2657.htm
[n2927]: http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2009/n2927.pdf
[N3648]: https://isocpp.org/files/papers/N3648.html
[N3649]: https://isocpp.org/files/papers/N3649.html
@ -980,7 +988,7 @@ References from C++23
### Additional Permitted Features
* `constexpr`
([n2235](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2235.pdf))
([n2235](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2007/n2235.pdf))
([n3652](https://isocpp.org/files/papers/N3652.html))
* Sized deallocation
@ -1087,14 +1095,14 @@ in HotSpot code because of the "no implicit boolean" guideline.)
* Avoid covariant return types.
* Avoid `goto` statements.
* Avoid `goto` statements.
### Undecided Features
This list is incomplete; it serves to explicitly call out some
features that have not yet been discussed.
* Trailing return type syntax for functions
* Trailing return type syntax for functions
([n2541](http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2541.htm))
* Variable templates
@ -1108,7 +1116,7 @@ features that have not yet been discussed.
* Rvalue references and move semantics
[ADL]: https://en.cppreference.com/w/cpp/language/adl
[ADL]: https://en.cppreference.com/w/cpp/language/adl
"Argument Dependent Lookup"
[ODR]: https://en.cppreference.com/w/cpp/language/definition