realm-js
realm-js copied to clipboard
WIP using JSI's NativeState class for faster access to native C++ object when Hermes enabled
What, How & Why?
This closes # ???
☑️ ToDos
- [ ] 📝 Changelog entry
- [ ] 📝
Compatibilitylabel is updated or copied from previous entry - [ ] 🚦 Tests
- [ ] 🔀 Executed flexible sync tests locally if modifying flexible sync
- [ ] 📦 Updated internal package version in consuming
package.jsons (if updating internal packages) - [ ] 📱 Check the React Native/other sample apps work if necessary
- [ ] 📝 Public documentation PR created or is not necessary
- [ ] 💥
Breakinglabel has been applied or is not necessary
If this PR adds or changes public API's:
- [ ] typescript definitions file is updated
- [ ] jsdoc files updated
- [ ] Chrome debug API is updated if API is available on React Native
@kraenhansen @RedBeard0531 I wanted to open this WIP to start a discussion around supporting the new NativeState API in Hermes master, which provides a way to store a C++ object on a JSI object and allows us to have much faster access to the native object when Hermes is enabled. It is not currently in an RN release, so the build will fail (you need to build RN from source to make it work).
It seems to work (just running the performance tests) but I am sure I'll have missed something as this stuff is complex!
The main gotcha here is that the NativeState API is not implemented for the JSCRuntime and instead throws a logic_error, so I am catching that and falling back to the old property method, as I couldn't find a nicer way to detect if Hermes is enabled.
Another gotcha is this will tie us to a minimum of whatever RN version includes this API. Not sure if that is an issue or if there's a good way to work around that if so.
OK adding the try/catch makes this like 20x slower with JSC (compared to just having the "catch" code there), lol, I guess that won't fly... maybe we could store a class static optional flag native_state_supported, then either set that to true or false depending on whether we hit a std::logic_error the first time we try to set_internal... then in future, skip the try catch and just run the appropriate block based on that flag. (Edit: see below comment for rough version)
Any better ideas @RedBeard0531?
I pushed a rough version of what I described above but I'll leave the comment intact in case the history gets confusing! https://github.com/realm/realm-js/pull/4871/commits/2a8ccebbab638c7ad36e18b595aea53f7bd7fb94