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

instanceof on host objects #1467

Closed
wcandillon opened this issue Aug 3, 2024 · 16 comments
Closed

instanceof on host objects #1467

wcandillon opened this issue Aug 3, 2024 · 16 comments
Labels
bug Something isn't working

Comments

@wcandillon
Copy link

wcandillon commented Aug 3, 2024

The following pattern:

declare var GPUQueue: {
  prototype: GPUQueue;
  new (): never;
};

interface GPUQueue {
 //...
}

Means that people should be able to write:

device.queue instanceof GPUQueue

Is it possible to support this syntax if device.queue is a host object?

@wcandillon wcandillon added the bug Something isn't working label Aug 3, 2024
@wcandillon
Copy link
Author

Closing has I realized that Hermes supports the Symbol.hasInstance syntax

@tmikov
Copy link
Contributor

tmikov commented Aug 4, 2024

First let me say that we strongly recommend against using HostObject, unless absolutely necessary. It is an anti-pattern and should ideally never be used. (Admittedly, it was the only option early on, but since then we added NativeState.)

In any case, you shouldn't need any tricks to do this. instanceof works by comparing obj.__proto__ to constructor.prototype, so as long as the host object's prototype is set correctly, it should work.

@wcandillon
Copy link
Author

wcandillon commented Aug 4, 2024

Could explain why NativeState is the better approach? If you love to see compelling examples that would show how we could benefit from this API.
cc @mrousavy as I am using his very compelling help for JSI host objects.

On our side, this is how we use it:
From the following TS:

interface Foo {
  bar(param?: string | number): bool;
}

We generate the following C++ code:

class Foo: HybridObject {
   
   bool bar(param: std::optional<std::variant<std::string, double>>) {
     return true;
  }
  
  void loadHybridMethods() override {
    registerHybridMethod("bar", &Foo::bar, this);
  }
}

This approach has done wonders for us as we code generate >90% of the work.

@mrousavy
Copy link
Contributor

mrousavy commented Aug 4, 2024

(@tmikov this is the nitro modules API I briefly mentioned on our discord convo, if you remember I had a similar HostObject but in Swift)

@tmikov
Copy link
Contributor

tmikov commented Aug 4, 2024

I have created some diagrams to illustrate the difference between HostObject and NativeState.

The first diagram shows how normal JS objects or classes work.

They are fairly efficient because the property dictionary is shared between all instances, methods are stored only once in the prototype, and everything is subject to property caching in the JSVM. Best case, a property access is a comparison and a direct access to the data slot in the object.

Additionally, the data is naturally managed by the GC.

objects-jsobj

The second diagram shows HostObject

The main problem here is that there is no property offset caching, so every access requires a C++ virtual call. Additionally, custom C++ logic is needed for an efficient implementation of property lookup. Additional custom complexity is needed to avoid duplicating methods for every instance of the object.

The second problem is that jsi::Value is probe to creating un-collectable GC cycles, so a lot of care is needed to manage the lifetimes.

objects-hostobj

The third diagram shows NativeState

NativeState is simply a way to attach "native data" to a JS object. All of the benefits of regular JS objects are preserved, very little custom C++ is required.

objects-nativestate

@mrousavy
Copy link
Contributor

mrousavy commented Aug 4, 2024

I see - thanks for the explanation. Just a few thoughts:

  1. HostObject supports custom getters/setters for properties - this is something we use a lot in favor of native functions as it is just a nicer API for the user. This is something that NativeState does not support, right?
  2. NativeState requires eager initialization of all methods and properties - so let's say we use a HostObject where we have 30 methods, but JS only uses 1, we have a bigger initial overhead because 30 methods need to be created and attached to NativeState - correct?

We built quite a few things with HostObjects already and performance was really really good. We took good care of optimizing property/method access and we cache a ton of things, and we even let JS(I) know about external (native) memory size - all handled automatically by nitro modules (the "framework"/library I'm working on).

So far, HostObject is working great for us. Dropping support for properties (getters/setters) is not worth the performance gains for us right now, but I still built a second implementation that uses NativeState instead of HostObject inside nitro modules - so far, it's unused. But it's there, just in case 😁

(@wcandillon maybe all Hybrid Objects that don't have properties can be NativeState, and once it has a property I can automatically generate HostObject code instead? should be completely transparent to the user and library author)

@mrousavy
Copy link
Contributor

mrousavy commented Aug 4, 2024

Continuing the thought experiment on a HostObject vs NativeState with 30 methods - we have a case where we run a function on each frame (so 60 or 120 times a second).

Inside this function, the user can access the current Camera to the 3D scene:

const callback = useFrameCallback((scene) => {
  const camera = scene.getCamera() // <-- camera is a jsi::HostObject
  camera.setPosition(...)
})

Now I haven't benchmarked this yet, but to me it sounds like returning a HostObject from the getCamera() function is faster than returning a NativeState, because the NativeState will have to eagerly initialize all methods first (setPosition, ...29 other methods) while HostObject doesn't.

We also can't really cache the result to getCamera() - it's either gonna look ugly to the user if he caches it himself (because all operations on getCamera() are under a native scope, it's not valid outside of useFrameCallback), or it's gonna be a long-lived jsi::Object on our native side - right?

@tmikov
Copy link
Contributor

tmikov commented Aug 4, 2024

HostObject supports custom getters/setters for properties - this is something we use a lot in favor of native functions as it is just a nicer API for the user. This is something that NativeState does not support, right?

NaiveState is a just way to associate native data with a JS object. No more, no less. It does not "support" neither properties nor methods. These concepts are orthogonal to it.

However ordinary JS already supports property getters and setters. Their implementation can be in C++ and can access NativeState.

  1. NativeState requires eager initialization of all methods and properties - so let's say we use a HostObject where we have 30 methods, but JS only uses 1, we have a bigger initial overhead because 30 methods need to be created and attached to NativeState - correct?

Allocating 30 instances of HostFunction and performing 30 property writes is very cheap. Plus, it is happening exactly once.

Why do you believe that this is a performance problem?

Ignoring the performance differences, the high level point is that NativeState allows easy integration of native data and code with existing objects, without changing how JS objects and classes already work. Subclassing, methods, reflection, etc, everything just works because it is the same.

HostObject, by comparison, requires a custom implementation in order to approximate the behavior of an object. Extra effort to implement property enumeration, subclassing, etc. Debugging gets harder, especially if you have instances from different vendors, because each implements things differently. Etc.

Of course software development is all about trade-offs. If the execution of the body of the method itself is expensive, the cost to fetch the method probably doesn't matter. If you have already built the infrastructure for HostObject, the convenience offered by NativeState is marginal. Etc.

So, you should, of course, do whatever is best for your case.

@tmikov
Copy link
Contributor

tmikov commented Aug 4, 2024

Continuing the thought experiment on a HostObject vs NativeState with 30 methods - we have a case where we run a function on each frame (so 60 or 120 times a second).

Inside this function, the user can access the current Camera to the 3D scene:

const callback = useFrameCallback((scene) => {
  const camera = scene.getCamera() // <-- camera is a jsi::HostObject
  camera.setPosition(...)
})

Now I haven't benchmarked this yet, but to me it sounds like returning a HostObject from the getCamera() function is faster than returning a NativeState, because the NativeState will have to eagerly initialize all methods first (setPosition, ...29 other methods) while HostObject doesn't.

We also can't really cache the result to getCamera() - it's either gonna look ugly to the user if he caches it himself (because all operations on getCamera() are under a native scope, it's not valid outside of useFrameCallback), or it's gonna be a long-lived jsi::Object on our native side - right?

Ideally, the methods should live in the prototype object. Creating the prototyoe should perform almost no work and it should happen exactly once.

That would be the natural way to define a JS class with or without NativeState.

It is also possible with HostObject, of course (not sure whether you are doing that), but requires extra complexity.

Perhaps I don't understand the problem.

@wcandillon
Copy link
Author

Is there a way for a JSI function to return a class? And if yes, would that help?

const Foo = getFoo();
const foo = new Foo();

On our side, the fact that classes are not "workletizable" would make using NativeState a show stopper. However Reanimated support for "workletizable" classes merged in main. So it is something we might consider in the future.

Right now we cogen from TS to C++ and here we would need to codegen from TS to JS and C.
But there would be some benefits to it.

As Marc mentioned, we would need to register the complete API upfront (in our case it is quite big).
And it does break some ergonomics: right now we do everything in C++ and it feels very transparent and in fact we had a lot of fun build such an API. But here we would need to revert to a C-style API where the native state is a resource handle and we have a huge number of functions in a global namespace that our JS code uses. But we will definitely consider this approach.

@mrousavy
Copy link
Contributor

mrousavy commented Aug 5, 2024

However ordinary JS already supports property getters and setters. Their implementation can be in C++ and can access NativeState.

Well, yea but that requires an additional JS layer, which we currently don't have at all.

Two downsides:

1. It will no longer be possible to access such properties from worklets, as those will now be runtime-specific functions (no longer native jsi::HostFunctionType)
2. It will require us to wrap the native calls in a JS based function, create all get and set props on the JS object (a secondary JS object), and then finally return it to the user again.

At this point, I don't think the performance gains are worth the added complexity and overheads. Again - I will definitely keep all of this in mind and have a NativeState implementation ready, but I think this only really works for objects that don't have properties (or readonly constants only).

EDIT: nvm, I can just get Object.defineProperty(..) from JS global in JSI, then call that, passing a get and set function. I thought this was just syntactic sugar but it seems like this is actually a JS feature.

So, you should, of course, do whatever is best for your case.

Makes total sense - yep. I really appreciate your insights here, this is what I have been curious about for a long time now.

I think my best solution is to continue using the HostObject approach as that works really well for us, I already have property/method caching and externaMemorySize, and everything's auto-generated.

I'll add a feature to support NativeState (either manually or automatically) when an object contains no properties. The user of my framework (nitro) can then choose himself if he wants to implement properties on the JS side, or not.

Thanks Tzvetan! See you at React Universe! 😄

@mrousavy
Copy link
Contributor

mrousavy commented Aug 5, 2024

(btw i just sent you an invite to the repo if you're curious what kind of sorcery I'm performing on HostObjects, all is in packages/react-native-nitro-modules/cpp/core or /jsi)

@mrousavy
Copy link
Contributor

mrousavy commented Aug 5, 2024

Is there a way for a JSI function to return a class? And if yes, would that help?

Isn't that just a function that applys a prototype?

@mrousavy
Copy link
Contributor

mrousavy commented Aug 5, 2024

Ideally, the methods should live in the prototype object. Creating the prototyoe should perform almost no work and it should happen exactly once.

Ohhhh- so wait maybe I am just now getting what you mean, are you saying this is how NativeState should be used?

  1. Initialize Prototype globally/only once:

    jsi::Object proto(runtime);
    proto.setProperty("someFunc", jsi::Function::createFromHostFunction([](...,
                                                                           const jsi::Value& thisValue,
                                                                           ...) {
      jsi::Object thisObj = thisValue.asObject(); // <-- get from `this`
      auto state = thisObj.getNativeState<MyNativeState>(); // <-- unwrap native state
      return state->someFunc(); // <-- actual C++ impl
    });
  2. Then each time we want to return a Camera jsi::Object with NativeState in getCamera, we just re-use the Prototype?

    jsi::Object camera(runtime);
    camera.setNativeState(cameraState);
    // TODO: Somehow apply proto to camera?
    // In JS we can do Object.setPrototype(..), but I don't think there's a direct JSI/C++ API for that,
    //   so I guess we just perform global() lookup on that method?
    return camera;

or am I misunderstanding what you said?

@wcandillon
Copy link
Author

wcandillon commented Aug 5, 2024 via email

@tmikov
Copy link
Contributor

tmikov commented Aug 7, 2024

@mrousavy here is an example of how NativeState can be used comfortably. There are two host functions that need to be exposed, which here are shown as JS for illustrative purposes.

https://gist.github.com/tmikov/3e6310abcd77ff8066297cdd4927b44d

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants