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

RJS-2852: Adding a CallInvoker-based scheduler for Core on React Native #6791

Merged
merged 11 commits into from
Jul 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .watchmanconfig
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@
"react-native/node_modules",
"packages/realm-web",
"packages/realm-web-integration-tests",
".wireit"
".wireit",
"packages/realm/.wireit",
"integration-tests/environments/react-native-test-app/.wireit"
Comment on lines +6 to +8
Copy link
Member Author

Choose a reason for hiding this comment

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

I got annoyed by these being indexed by watchman.

]
}
5 changes: 2 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,8 @@ realm.syncSession?.addProgressNotification(
* File format: generates Realms with format v24 (reads and upgrades file format v10).

### Internal
<!-- * Either mention core version or upgrade -->
<!-- * Using Realm Core vX.Y.Z -->
<!-- * Upgraded Realm Core from vX.Y.Z to vA.B.C -->
* Adding a CallInvoker-based scheduler for Core on React Native and removing the "flush ui queue" workaround. ([#6791](https://github.com/realm/realm-js/pull/6791))
* Refactors throwing uncaught exceptions from callbacks dispatched onto the event loop from C++ on React Native. ([#6772](https://github.com/realm/realm-js/issues/6772))

## 12.11.1 (2024-06-25)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1449,7 +1449,7 @@ SPEC CHECKSUMS:
ReactNativeHost: fd9d65f6ee7947f468537d35b3fefe8b2bf546da
ReactTestApp-DevSupport: 33a7ae9bf22161f359629a9607f5ef27e9c91fd0
ReactTestApp-Resources: d200e68756fa45c648f369210bd7ee0c14759f5a
RealmJS: b7ed9bcfaac0946479403b684d158d922f41384e
RealmJS: 298cc4766e917bd415fe0cfbfd1f4b198ab812f4
RNFS: 4ac0f0ea233904cb798630b3c077808c06931688
SocketRocket: abac6f5de4d4d62d24e11868d7a2f427e0ef940d
Yoga: ae3c32c514802d30f687a04a6a35b348506d411f
Expand Down
286 changes: 7 additions & 279 deletions package-lock.json

Large diffs are not rendered by default.

9 changes: 0 additions & 9 deletions packages/realm/bindgen/src/realm_js_jsi_helpers.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,9 @@
#include <realm_helpers.h>
#include <type_traits>

#include <flush_ui_queue_workaround.h>

namespace realm::js::JSI {
namespace {

struct FlushMicrotaskQueueGuard {
~FlushMicrotaskQueueGuard()
{
realm::js::flush_ui_workaround::flush_ui_queue();
}
};

namespace jsi = facebook::jsi;
template <typename Ref>
struct HostRefWrapper : jsi::HostObject {
Expand Down
2 changes: 0 additions & 2 deletions packages/realm/bindgen/src/templates/jsi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -652,8 +652,6 @@ function convertFromJsi(addon: JsiAddon, type: Type, expr: string): string {
{
_thread.assertOnSameThread();
auto& _env = ${addon.get()}->m_rt;
// TODO consider not flushing when calling back into JS from withing a JS->CPP call.
FlushMicrotaskQueueGuard guard;
return ${c(
type.ret,
`_cb->call(
Expand Down
2 changes: 1 addition & 1 deletion packages/realm/binding/android/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ endif()

add_library(realm-js-android-binding SHARED
../jsi/jsi_init.cpp
../jsi/flush_ui_queue_workaround.cpp
../jsi/react_scheduler.cpp
src/main/cpp/platform.cpp
src/main/cpp/jni_utils.cpp
src/main/cpp/io_realm_react_RealmReactModule.cpp
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@
#include <jsi/jsi.h>

#include "jsi_init.h"
#include "flush_ui_queue_workaround.h"
#include "react_scheduler.h"
#include "platform.hpp"
#include "jni_utils.hpp"

Expand Down Expand Up @@ -108,8 +108,8 @@ extern "C" JNIEXPORT void JNICALL Java_io_realm_react_RealmReactModule_setDefaul
realm::JsPlatformHelpers::default_realm_file_directory().c_str());
}

extern "C" JNIEXPORT void JNICALL Java_io_realm_react_RealmReactModule_injectCallInvoker(JNIEnv* env, jobject thiz,
jobject call_invoker)
extern "C" JNIEXPORT void JNICALL Java_io_realm_react_RealmReactModule_createScheduler(JNIEnv* env, jobject thiz,
jobject call_invoker)
{
// React Native uses the fbjni library for handling JNI, which has the concept of "hybrid objects",
// which are Java objects containing a pointer to a C++ object. The CallInvokerHolder, which has the
Expand All @@ -119,6 +119,9 @@ extern "C" JNIEXPORT void JNICALL Java_io_realm_react_RealmReactModule_injectCal

// 1. Get the Java object referred to by the mHybridData field of the Java holder object
auto callInvokerHolderClass = env->FindClass("com/facebook/react/turbomodule/core/CallInvokerHolderImpl");
if (!env->IsInstanceOf(call_invoker, callInvokerHolderClass)) {
throw std::invalid_argument("Expected call_invoker to be CallInvokerHolderImpl");
}
auto hybridDataField = env->GetFieldID(callInvokerHolderClass, "mHybridData", "Lcom/facebook/jni/HybridData;");
auto hybridDataObj = env->GetObjectField(call_invoker, hybridDataField);

Expand All @@ -136,14 +139,16 @@ extern "C" JNIEXPORT void JNICALL Java_io_realm_react_RealmReactModule_injectCal
// 4. Cast the mNativePointer back to its C++ type
auto nativePointer = reinterpret_cast<facebook::react::CallInvokerHolder*>(nativePointerValue);

// 5. Inject the JS call invoker for the workaround to use
realm::js::flush_ui_workaround::inject_js_call_invoker(nativePointer->getCallInvoker());
// 5. Create the scheduler from the JS call invoker
__android_log_print(ANDROID_LOG_VERBOSE, "Realm", "Creating scheduler");
realm::js::react_scheduler::create_scheduler(nativePointer->getCallInvoker());
}

extern "C" JNIEXPORT void JNICALL Java_io_realm_react_RealmReactModule_invalidateCaches(JNIEnv* env, jobject thiz)
{
// Disable the flush ui workaround
realm::js::flush_ui_workaround::reset_js_call_invoker();
__android_log_print(ANDROID_LOG_VERBOSE, "Realm", "Resetting scheduler");
// Reset the scheduler to prevent invocations using an old runtime
realm::js::react_scheduler::reset_scheduler();
__android_log_print(ANDROID_LOG_VERBOSE, "Realm", "Invalidating caches");
#if DEBUG
// Immediately close any open sync sessions to prevent race condition with new
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ public boolean injectModuleIntoJSGlobal() {
}

CallInvokerHolder jsCallInvokerHolder = reactContext.getCatalystInstance().getJSCallInvokerHolder();
injectCallInvoker(jsCallInvokerHolder);
createScheduler(jsCallInvokerHolder);

// Get the javascript runtime and inject our native module with it
JavaScriptContextHolder jsContext = reactContext.getJavaScriptContextHolder();
Expand All @@ -103,11 +103,9 @@ public boolean injectModuleIntoJSGlobal() {
private native void injectModuleIntoJSGlobal(long runtimePointer);

/**
* Passes the React Native jsCallInvokerHolder over to C++ so we can setup our UI queue flushing.
* This is needed as a workaround for https://github.com/facebook/react-native/issues/33006
* where we call the invokeAsync method to flush the React Native UI queue whenever we call from C++ to JS.
* Passes the React Native jsCallInvokerHolder over to C++ so we can setup a scheduler.
*/
private native void injectCallInvoker(CallInvokerHolder callInvoker);
private native void createScheduler(CallInvokerHolder callInvoker);

private native void invalidateCaches();
}
13 changes: 5 additions & 8 deletions packages/realm/binding/apple/RealmReactModule.mm
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
#import <objc/runtime.h>

#import "jsi/jsi_init.h"
#import "flush_ui_queue_workaround.h"
#import "jsi/react_scheduler.h"

// the part of the RCTCxxBridge private class we care about
@interface RCTBridge (Realm_RCTCxxBridge)
Expand All @@ -54,7 +54,8 @@ - (dispatch_queue_t)methodQueue {
}

- (void)invalidate {
realm::js::flush_ui_workaround::reset_js_call_invoker();
// Reset the scheduler to prevent invocations using an old runtime
realm::js::react_scheduler::reset_scheduler();
#if DEBUG
// Immediately close any open sync sessions to prevent race condition with new
// JS thread when hot reloading
Expand All @@ -74,12 +75,8 @@ - (void)dealloc {
RCTBridge* bridge = [RCTBridge currentBridge];
auto &rt = *static_cast<facebook::jsi::Runtime *>(bridge.runtime);

// Since https://github.com/facebook/react-native/pull/43396 this should only be needed when bridgeless is not enabled.
// but unfortunately, that doesn't seem to be the case.
// See https://github.com/facebook/react-native/pull/43396#issuecomment-2178586017 for context
// If it was, we could use the enablement of "microtasks" to avoid the overhead of calling the invokeAsync on every call from C++ into JS.
// if (!facebook::react::ReactNativeFeatureFlags::enableMicrotasks()) {
realm::js::flush_ui_workaround::inject_js_call_invoker([bridge jsCallInvoker]);
// Create a scheduler wrapping the call invoker and pass this to realm core
realm::js::react_scheduler::create_scheduler([bridge jsCallInvoker]);

auto exports = jsi::Object(rt);
realm_jsi_init(rt, exports);
Expand Down
62 changes: 0 additions & 62 deletions packages/realm/binding/jsi/flush_ui_queue_workaround.cpp

This file was deleted.

98 changes: 98 additions & 0 deletions packages/realm/binding/jsi/react_scheduler.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
////////////////////////////////////////////////////////////////////////////
//
// Copyright 2024 Realm Inc.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//
////////////////////////////////////////////////////////////////////////////

#include "react_scheduler.h"

#include <realm/object-store/util/scheduler.hpp>

#include <ReactCommon/CallInvoker.h>
#include <ReactCommon/SchedulerPriority.h>

#include <thread>

using Scheduler = realm::util::Scheduler;

namespace {

std::shared_ptr<Scheduler> scheduler_{};

class ReactScheduler : public realm::util::Scheduler {
Copy link
Member

Choose a reason for hiding this comment

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

This is very minor, but this type and the global variable should be in an anonymous namespace so that the symbols are not exported.

public:
ReactScheduler(std::shared_ptr<facebook::react::CallInvoker> js_call_invoker)
: m_js_call_invoker(js_call_invoker)
{
}

bool is_on_thread() const noexcept override
{
return m_id == std::this_thread::get_id();
}

bool is_same_as(const Scheduler* other) const noexcept override
{
auto o = dynamic_cast<const ReactScheduler*>(other);
return (o && (o->m_js_call_invoker == m_js_call_invoker));
}

bool can_invoke() const noexcept override
{
return true;
}

void invoke(realm::util::UniqueFunction<void()>&& func) override
{
m_js_call_invoker->invokeAsync(
// Using normal priority to avoid blocking rendering, user-input and other higher priority tasks
facebook::react::SchedulerPriority::NormalPriority, [ptr = func.release()] {
(realm::util::UniqueFunction<void()>(ptr))();
});
}

private:
std::shared_ptr<facebook::react::CallInvoker> m_js_call_invoker;
std::thread::id m_id = std::this_thread::get_id();
};

std::shared_ptr<Scheduler> get_scheduler()
{
if (scheduler_) {
REALM_ASSERT(scheduler_->is_on_thread());
return scheduler_;
}
else {
return Scheduler::make_platform_default();
}
}

} // namespace

namespace realm::js::react_scheduler {


void create_scheduler(std::shared_ptr<facebook::react::CallInvoker> js_call_invoker)
{
scheduler_ = std::make_shared<ReactScheduler>(js_call_invoker);
Scheduler::set_default_factory(get_scheduler);
}

void reset_scheduler()
{
scheduler_.reset();
}

} // namespace realm::js::react_scheduler
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,7 @@

#include <ReactCommon/CallInvoker.h>

namespace realm::js::flush_ui_workaround {
void inject_js_call_invoker(std::shared_ptr<facebook::react::CallInvoker> js_invoker);
void reset_js_call_invoker();
void flush_ui_queue();
} // namespace realm::js::flush_ui_workaround
namespace realm::js::react_scheduler {
void create_scheduler(std::shared_ptr<facebook::react::CallInvoker> js_invoker);
void reset_scheduler();
} // namespace realm::js::react_scheduler
Loading