RFR: 8234773: Fix ThreadsSMRSupport::_bootstrap_list

[David Holmes]

On Mon, 4 Jan 2021 01:29:10 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

> Please review this change to ThreadsList and the singleton _bootstrap_list.
> ThreadsList is made noncopyable, with _bootstrap_list changed to be direct
> initialized to avoid referencing the copy constructor.  The ThreadsList
> constructor now uses a static array for the 0-entry case, so that static
> ctor/dtor of _bootstrap_list doesn't involve C-heap allocation.
> 
> Testing:
> mach5 tier1
> 
> Local (linux-x64) build with -fno-elide-constructors and ran hotspot:tier1.
> This has a failure unrelated to this change:
> compiler/intrinsics/klass/CastNullCheckDroppingsTest.java fails with
> #  Internal Error (../../src/hotspot/share/jfr/utilities/jfrVersionSystem.inline.hpp:98), pid=31086, tid=31713
> #  assert(node->_live) failed: invariant
> Filed new bug: https://bugs.openjdk.java.net/browse/JDK-8259036

Hi Kim,
Overall the fix looks good. I have one comment on the direct initialization part (after reading up on direct initialization).

Thanks,
David

src/hotspot/share/runtime/threadSMR.cpp line 79:

> 77: 
> 78: // The bootstrap list is empty and cannot be freed.
> 79: ThreadsList ThreadsSMRSupport::_bootstrap_list{0};

To anyone unfamiliar with the details of direct initialization this is very obscure. I don't know how to interpret this given a ThreadsList has four member fields. Wouldn't `{}` have the same effect of implicitly zero-initializing all the fields?
Stylistically I find this syntax jarring, an equals sign in there would look far more natural to me. :(

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

Marked as reviewed by dholmes (Reviewer).

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