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

Fetch array length before dropping vm access #11241

Merged
merged 1 commit into from
Nov 20, 2020
Merged

Fetch array length before dropping vm access #11241

merged 1 commit into from
Nov 20, 2020

Conversation

DanHeidinga
Copy link
Member

Rather than dropping access, only to acquire and release it
during the env->GetArrayLength call, use the macro to
avoid the extra in and out of vm access

Signed-off-by: Dan Heidinga heidinga@redhat.com

@DanHeidinga
Copy link
Member Author

Minor change, found while reviewing something else

@gacholio
Copy link
Contributor

jenkins test sanity zlinux jdk15

fjeremic
fjeremic requested changes Nov 20, 2020
View reviewed changes
Copy link
Contributor

@fjeremic fjeremic left a comment

Choose a reason for hiding this comment

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

The first line should be less than 70 characters, where reasonable, and should be written in sentence case preferably not ending in a period. Leave a blank line between the first line and the message body.

[1] https://github.com/eclipse/openj9/blob/master/CONTRIBUTING.md#commit-guidelines

@gacholio
Copy link
Contributor

@DanHeidinga Please wait until testing completes before fixing the title in the commit.

@DanHeidinga
Copy link
Member Author

@DanHeidinga Please wait until testing completes before fixing the title in the commit.

This seems unnecessarily pedantic and adds additional coordination overhead onto a simple PR. We've gone form a straight test+merge, to a couple of head offs. :(

@gacholio
Copy link
Contributor

Lots of failed tests.

@gacholio
Copy link
Contributor

Seems likely related:

13:17:16  java.lang.IndexOutOfBoundsException
13:17:16  	at java.base/jdk.internal.misc.Unsafe.defineAnonymousClass0(Native Method)
13:17:16  	at java.base/jdk.internal.misc.Unsafe.defineAnonymousClass(Unsafe.java:1624)
13:17:16  	at org.openj9.test.unsafe.TestUnsafeAccess.testDefineAnonymousClass(TestUnsafeAccess.java:344)

@DanHeidinga
Fetch array length before dropping vm access
7917f50 
Rather than dropping access, only to acquire and release it
during the `env->GetArrayLength` call, use the macro to
avoid the extra in and out of vm access

Signed-off-by: Dan Heidinga <heidinga@redhat.com>
@DanHeidinga
Copy link
Member Author

jenkins test sanity zlinux jdk15

@DanHeidinga
Copy link
Member Author

I missed unwrapping the JNI ref

@gacholio gacholio changed the title Fetch array length before dropping vm access. Fetch array length before dropping vm access Nov 20, 2020
@gacholio gacholio merged commit 749f941 into eclipse-openj9:master Nov 20, 2020
@DanHeidinga DanHeidinga deleted the djh/unsafejni branch November 20, 2020 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fjeremic fjeremic fjeremic approved these changes

@gacholio gacholio gacholio approved these changes

Assignees
No one assigned
Labels
comp:vm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@DanHeidinga @gacholio @fjeremic