RFR: 8234773: Fix ThreadsSMRSupport::_bootstrap_list

Kim Barrett kim.barrett at oracle.com
Mon Jan 4 12:09:04 UTC 2021


> On Jan 3, 2021, at 9:16 PM, David Holmes <dholmes at openjdk.java.net> wrote:
> […]
> 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. :(

ThreadsList is not an aggregate; it has a constructor - `ThreadsList(int)`.
The proposed change calls that 1-arg constructor with an argument of 0.

Are you saying you would prefer

ThreadsList ThreadsSMRSupport::_bootstrap_list = { 0 };

This is copy-list-initialization, again because `ThreadsList` is not an
aggregate. In this particular case, it would end up doing the same thing as
direct-initialization, i.e. call the 1-arg constructor.

However, if that constructor were `explicit`, the initialization is
ill-formed and compilation should fail. And that constructor ought to be
`explicit`. HotSpot code is unfortunately *really* bad about not using
`explicit` where it ought to be used though.

Since I'm touching code in this area and have been reminded of it, I'm
planning to change the constructor to be `explicit`.

> Marked as reviewed by dholmes (Reviewer).

Thanks.

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




More information about the hotspot-runtime-dev mailing list