RFR: 8274322: Problems with oopDesc construction [v2]
Stefan Karlsson
stefank at openjdk.java.net
Thu Sep 30 11:22:45 UTC 2021
On Wed, 29 Sep 2021 06:57:10 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:
>> Please review this change to the default constructor for markWord and
>> associated "change" to construction of oopDesc.
>>
>> The current code never invokes the constructor for oopDesc or any of its
>> derived classes. For that to be permissible according to the Standard,
>> those classes must be trivially default constructible. And for that to be
>> the case, the markWord default constructor must be trivial.
>>
>> This change consists of three parts.
>>
>> (1) The markWord default constructor is changed to be trivial, so the
>> default constructors for oopDesc and classes derived from it will also be
>> trivial. It wasn't previously trivial because the mechanism for making it so
>> (a default definition) is a C++11 feature that wasn't yet supported when the
>> previous constructor was defined.
>>
>> (2) This change also adds static asserts to verify the relevant classes have
>> trivial default constructors, to prevent later changes from unintentionally
>> breaking this.
>>
>> (3) This change also makes oopDesc noncopyable, to prevent inadvertent usage
>> of these operations that don't make any sense.
>>
>> A different approach would be to always use placement new with an
>> appropriate constructor to perform the initialization, perhaps encapsulated
>> in factory functions. I did some exploration in that direction. It's a much
>> larger and more complex change, though the final behavior (use constructors
>> for initialization) is simpler.
>>
>> Testing:
>> tier1
>
> Kim Barrett has updated the pull request incrementally with one additional commit since the last revision:
>
> dholmes review
Marked as reviewed by stefank (Reviewer).
src/hotspot/share/oops/markWord.hpp line 79:
> 77:
> 78: // It is critical for performance that this class be trivially
> 79: // destructable, copyable, and assignable.
Given the comment, would it make sense to also explicitly mark them as `= default`?
-------------
PR: https://git.openjdk.java.net/jdk/pull/5729
More information about the hotspot-dev
mailing list