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

No "garbage-free" way to interact with the library #68

Open
dustContributor opened this issue May 3, 2024 · 2 comments
Open

No "garbage-free" way to interact with the library #68

dustContributor opened this issue May 3, 2024 · 2 comments

Comments

@dustContributor
Copy link

dustContributor commented May 3, 2024

Hi! I noticed something while using these bindings.

Having LWJGL as a reference, it provides usually a couple of overloads for every function. In particular it provides both wrapped and unwrapped versions of a native binding. It looks like this:

// I believe LWJGL prefixes all of these "direct" calls with n
public static native long ndoSomethingOnNative(long address);

public static ByteBuffer doSomethingOnNative(ByteBuffer data) {
  var result = ndoSomethingOnNative(addressOf(data)); // unwrap ByteBuffer argument
  return wrap(result); // wrap result in ByteBuffer
}

This way, if you do not want to go through object manipulation, because you're handling the long addresses directly, you have the chance to do so.

Now lets look at PxActor user data functions:

    public NativeObject getUserData() {
        checkNotNull();
        return NativeObject.wrapPointer(_getUserData(address));
    }
    private static native long _getUserData(long address);
    public void setUserData(NativeObject value) {
        checkNotNull();
        _setUserData(address, value.getAddress());
    }
    private static native void _setUserData(long address, long value);

While I can get the object's address field just fine with getAddress(), I am unable to directly call either _getUserData or _setUserData due their visibility.

Moreover, I am forced to create an object on every single call of these methods. getUserData may very well be a high frequency method to call.

Now, this limitation is propagated in the design of the whole library, you can find many cases such as this. Where you could, if it were visible, have a single PxSomething instance created, and just wrap/unwrap direct pointers mutating that lone instance's address field, instead of having to recreate those PxSomething objects on every single access.

In this particular case I'd use the native _setUserData function for something more pragmatic. I do not want to actually set an object reference there, I just want to set an int32 index. So I'd use it like:

PxActor o = // etc.
int entityId = // some entity id
PxActor._setUserData(o.getAddress(), entityId);
// and to use it, no objects created in either case
int entityId = (int)PxActor._getUserData(o.getAddress());

I do not believe this could apply on every single instance where a native call is issued, but I can imagine plenty of cases where object creation could be avoided if you could just access the native functions "directly" using long back and forth, and just manually mutating the address field of a single PxSomething object to manipulate the memory from Java.

Also I do know that physx-jni is very careful on how it uses objects and they never cross to the native side, so in theory escape analysis should be able to work its magic. However, it popped out to me since in particular the userData ID seemed a "simple" use case I couldn't emulate with just primitive usage with the API as it is.

My experience with PhysX is really barebones (and my project is nowhere near a serious one, just hobby things) so I cant comment on better ways of doing exactly what I wanted to do, but I hope it illustrates the kind of use case I'm thinking of.

@liskar-dev
Copy link

I believe no garbage will be allocated on heap with calls like these:

int entityId = (int) actor.getUserData().getAddress();
actor.setUserData(NativeObject.wrapPointer(entityId));

because of Escape Analysis mechanism in the Hotspot, you can read more on it here: https://shipilev.net/jvm/anatomy-quarks/18-scalar-replacement/

@fabmax
Copy link
Owner

fabmax commented Nov 10, 2024

Hmm I opted against making the unwrapped functions public because I thought they would clutter up the API. However, since the unwrapped functions are all static it shouldn't hurt too much because stuff like auto-complete should be smart enough to not display static functions when dealing with an object.

Another option I just thought about is to move the unwrapped functions into an inner class, something like this:

public class PxActor {
    public NativeObject getUserData() {
        checkNotNull();
        return NativeObject.wrapPointer(unsafe.getUserData(address));
    }
    // ...

    public static class unsafe {
        public static native long getUserData(long address);
        // ...
    }
}

and then you could do:

int entityId = (int) PxActor.unsafe.getUserData(o.getAddress());

Not sure if this comes with a performance penalty though

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

No branches or pull requests

3 participants