Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

VarHandles class initialization #3928

Merged
merged 1 commit into from
Jan 8, 2019
Merged

VarHandles class initialization #3928

merged 1 commit into from
Jan 8, 2019

Conversation

pdbain-ibm
Copy link
Contributor

Initialize the class when creating a VårHandle to a static field. This change
is to match the behaviour of the reference implementation.

Signed-off-by: Peter Bain peter_bain@ca.ibm.com

@pdbain-ibm pdbain-ibm mentioned this pull request Dec 5, 2018
Closed
@DanHeidinga
Copy link
Member

@pdbain-ibm Can you drop the "Version 2" from the commit messages? There's no commits with "Version 1" in them related to this space

@pdbain-ibm pdbain-ibm changed the title VarHandles class initialization, version 2 VarHandles class initialization Dec 11, 2018
@pdbain-ibm
Copy link
Contributor Author

pdbain-ibm commented Dec 11, 2018 •
edited
Loading

@DanHeidinga In reply to
#3928 (comment), to clarify, this is a replacement for #192 WIP: VarHandles class initializaiton.

@DanHeidinga
Copy link
Member

Which was never merged. Having a Version 2 in the git changelog is unnecessary

@DanHeidinga DanHeidinga self-assigned this Jan 7, 2019
@pdbain-ibm pdbain-ibm force-pushed the varhandle branch 2 times, most recently from adb77e3 to 4a0d104 Compare January 7, 2019 17:49
DanHeidinga
DanHeidinga requested changes Jan 7, 2019
View reviewed changes
Copy link
Member

@DanHeidinga DanHeidinga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pdbain-ibm there's a bunch of code here that can be removed now as well. Can you also update the commit comment to remove the "Version 2" reference and remove the å as well?

@pdbain-ibm
VarHandles class initialization
17f12b0 
Initialize the class when creating a VarHandle to a static field.  This change
is to match the behaviour of the reference implementation.

Remove unused code and imports.

This replaces obsolete changeset eclipse-openj9#192

Signed-off-by: Peter Bain <peter_bain@ca.ibm.com>
@JasonFengJ9
Copy link
Member

This PR is going to fix #3205.

org/openj9/test/varhandle/StaticFieldVarHandleTests.java:testFinalField() would need to be updated to remove StaticHelper.reset(); at https://github.com/eclipse/openj9/blob/master/test/functional/Java9andUp/src/org/openj9/test/varhandle/StaticFieldVarHandleTests.java#L2271.
This verifies that StaticFieldVarHandle getter initializes the static fields if they haven't been initialized before.

DanHeidinga
DanHeidinga approved these changes Jan 7, 2019
View reviewed changes
Copy link
Member

@DanHeidinga DanHeidinga left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@DanHeidinga
Copy link
Member

Jenkins test sanity zlinux jdk11

@DanHeidinga
Copy link
Member

Jenkins test extended xlinux jdk11

@JasonFengJ9
Copy link
Member

The test change mentioned meant to support this fix but not required for this PR.
Will open another PR instead.

@JasonFengJ9 JasonFengJ9 mentioned this pull request Jan 7, 2019
Merged
@DanHeidinga DanHeidinga merged commit 46dbded into eclipse-openj9:master Jan 8, 2019
@JasonFengJ9 JasonFengJ9 mentioned this pull request Jan 9, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DanHeidinga DanHeidinga DanHeidinga approved these changes

Assignees

@DanHeidinga DanHeidinga

Labels
comp:vm
Projects
None yet
Milestone
Release 0.12.0 (Java 8, 11 Jan refresh)
Development

Successfully merging this pull request may close these issues.
4 participants
@pdbain-ibm @DanHeidinga @JasonFengJ9 @pshipton