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.

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

[WIP] Remove unsafe object scan. #631

Closed
wants to merge 1 commit into from
Closed

[WIP] Remove unsafe object scan. #631

wants to merge 1 commit into from

Conversation

mukel
Copy link

@mukel mukel commented Mar 14, 2021

[WIP] Use JNI to compute field offsets instead of scanning the objects with Unsafe.

@httpdigest
Copy link
Member

The reason for the scanning is because the name of the Buffer's "address" field is not known (it can differ in different JREs, which don't use the OpenJDK class library).
Apart from that: Even if the field name was known in advance, there wouldn't be a reason to use JNI, which in your case simply looks up the sun.misc.Unsafe's objectFieldOffset() method in native code and calling that method via JNI on the supplied Unsafe instance.
That could've just as well been implemented more directly in Java.
But, like I said: We cannot do this. We have to scan.

@mukel
Copy link
Author

mukel commented Mar 14, 2021

Scanning objects like that is very fragile, field offsets are just a token/cookie, the physical location remains an implementation detail. The VM may decide the shuffle fields (packing), or even remove fields that are always constant or unused and provide a "fake" offset instead.
The issue was originally reported here. Espresso is an alternative JVM, we use the OpenJDK class library, but we use a different object model/layout which sometimes breaks spec-bending code.
Are you referring to the Android class library? Which alternative class library is incompatible?
Sorry for the sloppy workaround, I'd really like to give this a few more cycles, probably I can suggest an alternative, less invasive workaround to the scanning.

@mukel
Copy link
Author

mukel commented Mar 14, 2021

Just read https://blog.lwjgl.org/memory-management-in-lwjgl-3/
To get the DirectByteBuffer address offset, would you accept rather scanning all fields to find it?
Peeking known fields in a type-safe manner to find a magic constant is an improvement (spec-compliant) over peeking with Unsafe.

@Spasi
Copy link
Member

Spasi commented Mar 14, 2021

Hey @mukel, I think I know how to implement this in a compliant way. Could you please close this PR and open an issue instead?

Thanks!

@mukel
Copy link
Author

mukel commented Mar 14, 2021

Opened #632

@mukel mukel closed this Mar 14, 2021
@mukel mukel deleted the remove-unsafe-object-scan branch March 14, 2021 21:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants