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