8389452: Inherited @Contended annotation corrupts concrete value class layout - #32470
8389452: Inherited @Contended annotation corrupts concrete value class layout#32470caspernorrbin wants to merge 2 commits into
Conversation
|
👋 Welcome back cnorrbin! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
@caspernorrbin The following label will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command. |
|
The total number of required reviews for this PR has been set to 2 based on the presence of this label: |
Webrevs
|
fparain
left a comment
There was a problem hiding this comment.
Changes look good to me.
The fix just needs the update to the Javadoc of @contended we discussed.
johan-sjolen
left a comment
There was a problem hiding this comment.
This LGTM, but please address Frederic's comment.
|
@caspernorrbin |
fparain
left a comment
There was a problem hiding this comment.
LGTM
Thanks for the Javadoc update
Hi everyone,
The
@Contendednotation is not supported on value classes. The intended behavior is for the annotation to be ignored without affecting the class layout. However, value classes are currently still marked as contended internally. When another class inherits from a value class marked as contended, this causes padding to be inserted between the inherited fields and the subclass fields, which causes a crash on debug builds.To fix this, I have changed the handling so that a class is only marked as contended if it is an identity class. Since value classes cannot be contended, filtering the annotation when it is processed avoids carrying an invalid state that would require extra checks later on. With this change, contended and non-contended value classes behave identically, and inheriting from a value class annotated with
@Contendedno longer introduces padding.Testing:
Progress
Issue
Reviewers
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/32470/head:pull/32470$ git checkout pull/32470Update a local copy of the PR:
$ git checkout pull/32470$ git pull https://git.openjdk.org/jdk.git pull/32470/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 32470View PR using the GUI difftool:
$ git pr show -t 32470Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/32470.diff
Using Webrev
Link to Webrev Comment