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

perf: Only enable -O2 in release, use -O0 in debug #356

Merged
merged 12 commits into from
Nov 19, 2024
7 changes: 6 additions & 1 deletion packages/nitrogen/src/syntax/kotlin/KotlinCxxBridgedType.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ export class KotlinCxxBridgedType implements BridgedType<'kotlin', 'c++'> {
name: 'NitroModules/JArrayBuffer.hpp',
space: 'system',
})
imports.push({
language: 'c++',
name: 'NitroModules/JUnit.hpp',
space: 'system',
})
break
case 'promise':
imports.push({
Expand Down Expand Up @@ -528,7 +533,7 @@ export class KotlinCxxBridgedType implements BridgedType<'kotlin', 'c++'> {
[&]() {
jni::local_ref<JPromise::javaobject> __promise = JPromise::create();
${parameterName}->addOnResolvedListener([=]() {
__promise->cthis()->resolve(nullptr);
__promise->cthis()->resolve(JUnit::instance());
});
${parameterName}->addOnRejectedListener([=](const std::exception& __error) {
auto __jniError = jni::JCppException::create(__error);
Expand Down
11 changes: 10 additions & 1 deletion packages/react-native-nitro-image/android/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,18 @@ android {

externalNativeBuild {
cmake {
cppFlags "-O2 -frtti -fexceptions -Wall -fstack-protector-all"
cppFlags "-frtti -fexceptions -Wall -Wextra -fstack-protector-all"
arguments "-DANDROID_STL=c++_shared"
abiFilters (*reactNativeArchitectures())

buildTypes {
debug {
cppFlags "-O1 -g"
}
release {
cppFlags "-O2"
}
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ namespace margelo::nitro::image { class HybridBaseSpec; }
#include "JCar.hpp"
#include <NitroModules/ArrayBuffer.hpp>
#include <NitroModules/JArrayBuffer.hpp>
#include <NitroModules/JUnit.hpp>
#include "HybridChildSpec.hpp"
#include "JHybridChildSpec.hpp"
#include "HybridBaseSpec.hpp"
Expand Down Expand Up @@ -462,7 +463,7 @@ namespace margelo::nitro::image {
auto __result = method(_javaPart, [&]() {
jni::local_ref<JPromise::javaobject> __promise = JPromise::create();
promise->addOnResolvedListener([=]() {
__promise->cthis()->resolve(nullptr);
__promise->cthis()->resolve(JUnit::instance());
});
promise->addOnRejectedListener([=](const std::exception& __error) {
auto __jniError = jni::JCppException::create(__error);
Expand Down
11 changes: 10 additions & 1 deletion packages/react-native-nitro-modules/android/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,18 @@ android {

externalNativeBuild {
cmake {
cppFlags "-O2 -frtti -fexceptions -Wall -fstack-protector-all"
cppFlags "-frtti -fexceptions -Wall -Wextra -fstack-protector-all"
arguments "-DANDROID_STL=c++_shared"
abiFilters (*reactNativeArchitectures())

buildTypes {
debug {
cppFlags "-O1 -g"
}
release {
cppFlags "-O2"
}
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,23 @@

#include "Promise.hpp"
#include <fbjni/fbjni.h>
#include <mutex>

namespace margelo::nitro {

using namespace facebook;

struct JOnResolvedCallback : public jni::JavaClass<JOnResolvedCallback> {
static auto constexpr kJavaDescriptor = "Lcom/margelo/nitro/core/Promise$OnResolvedCallback;";
void onResolved(jni::alias_ref<jni::JObject> result) const {
void onResolved(const jni::alias_ref<jni::JObject>& result) const {
static const auto method = javaClassLocal()->getMethod<void(jni::alias_ref<jni::JObject>)>("onResolved");
method(self(), result);
}
};

struct JOnRejectedCallback : public jni::JavaClass<JOnRejectedCallback> {
static auto constexpr kJavaDescriptor = "Lcom/margelo/nitro/core/Promise$OnRejectedCallback;";
void onRejected(jni::alias_ref<jni::JThrowable> error) const {
void onRejected(const jni::alias_ref<jni::JThrowable>& error) const {
static const auto method = javaClassLocal()->getMethod<void(jni::alias_ref<jni::JThrowable>)>("onRejected");
method(self(), error);
}
Expand Down Expand Up @@ -57,12 +58,14 @@ class JPromise final : public jni::HybridClass<JPromise> {

public:
void resolve(jni::alias_ref<jni::JObject> result) {
std::unique_lock lock(_mutex);
_result = jni::make_global(result);
for (const auto& onResolved : _onResolvedListeners) {
onResolved(_result);
}
}
void reject(jni::alias_ref<jni::JThrowable> error) {
std::unique_lock lock(_mutex);
_error = jni::make_global(error);
for (const auto& onRejected : _onRejectedListeners) {
onRejected(_error);
Expand All @@ -71,6 +74,7 @@ class JPromise final : public jni::HybridClass<JPromise> {

public:
void addOnResolvedListener(OnResolvedFunc&& onResolved) {
std::unique_lock lock(_mutex);
if (_result != nullptr) {
// Promise is already resolved! Call the callback immediately
onResolved(_result);
Expand All @@ -80,6 +84,7 @@ class JPromise final : public jni::HybridClass<JPromise> {
}
}
void addOnRejectedListener(OnRejectedFunc&& onRejected) {
std::unique_lock lock(_mutex);
if (_error != nullptr) {
// Promise is already rejected! Call the callback immediately
onRejected(_error);
Expand All @@ -91,23 +96,25 @@ class JPromise final : public jni::HybridClass<JPromise> {

private:
void addOnResolvedListenerJava(jni::alias_ref<JOnResolvedCallback> callback) {
std::unique_lock lock(_mutex);
if (_result != nullptr) {
// Promise is already resolved! Call the callback immediately
callback->onResolved(_result);
} else {
// Promise is not yet resolved, put the listener in our queue.
auto sharedCallback = jni::make_global(callback);
_onResolvedListeners.push_back([=](const auto& result) { sharedCallback->onResolved(result); });
_onResolvedListeners.emplace_back([=](const auto& result) { sharedCallback->onResolved(result); });
}
}
void addOnRejectedListenerJava(jni::alias_ref<JOnRejectedCallback> callback) {
std::unique_lock lock(_mutex);
if (_error != nullptr) {
// Promise is already rejected! Call the callback immediately
callback->onRejected(_error);
} else {
// Promise is not yet rejected, put the listener in our queue.
auto sharedCallback = jni::make_global(callback);
_onRejectedListeners.push_back([=](const auto& error) { sharedCallback->onRejected(error); });
_onRejectedListeners.emplace_back([=](const auto& error) { sharedCallback->onRejected(error); });
}
}

Expand All @@ -121,6 +128,7 @@ class JPromise final : public jni::HybridClass<JPromise> {
jni::global_ref<jni::JThrowable> _error;
std::vector<OnResolvedFunc> _onResolvedListeners;
std::vector<OnRejectedFunc> _onRejectedListeners;
std::mutex _mutex;

public:
static void registerNatives() {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
//
// JUnit.hpp
// react-native-nitro
//
// Created by Marc Rousavy on 19.11.24.
//

#pragma once

#include "NitroLogger.hpp"
#include <fbjni/fbjni.h>

namespace margelo::nitro {

using namespace facebook;

/**
* Represents a `Unit` from Kotlin.
* This is similar to `void` for Java, but is actually an `Object`.
*/
class JUnit final {
public:
/**
* Gets the shared instance to `Unit`. This is always a static global.
*/
static jni::alias_ref<jni::JObject> instance() {
static jni::global_ref<jni::JObject> sharedInstance = nullptr;
if (sharedInstance == nullptr) {
jni::alias_ref<jni::JClass> clazz = jni::findClassStatic("java/lang/Object");
jni::JConstructor<jobject()> constructor = clazz->getConstructor<jobject()>();
jni::local_ref<jobject> instance = clazz->newObject(constructor);
sharedInstance = jni::make_global(instance);
}
return sharedInstance;
}
};

} // namespace margelo::nitro
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import com.facebook.jni.HybridData
import com.facebook.proguard.annotations.DoNotStrip
import kotlinx.coroutines.CoroutineScope
import kotlinx.coroutines.Dispatchers
import kotlinx.coroutines.async
import kotlinx.coroutines.launch
import kotlin.concurrent.thread
import kotlin.coroutines.resume
Expand All @@ -26,21 +25,6 @@ import kotlin.coroutines.suspendCoroutine
@Keep
@DoNotStrip
class Promise<T> {
@Keep
@DoNotStrip
fun interface OnResolvedCallback<T> {
@Keep
@DoNotStrip
fun onResolved(result: T)
}
@Keep
@DoNotStrip
fun interface OnRejectedCallback {
@Keep
@DoNotStrip
fun onRejected(error: Throwable)
}

@Keep
@DoNotStrip
private val mHybridData: HybridData
Expand Down Expand Up @@ -79,16 +63,20 @@ class Promise<T> {
* Add a continuation listener to this `Promise<T>`.
* Once the `Promise<T>` resolves, the [listener] will be called.
*/
fun then(listener: OnResolvedCallback<T>): Promise<T> {
addOnResolvedListener(listener)
fun then(listener: (result: T) -> Unit): Promise<T> {
addOnResolvedListener { boxedResult ->
@Suppress("UNCHECKED_CAST")
val result = boxedResult as? T ?: throw Error("Failed to cast Object to T!")
listener(result)
}
return this
}

/**
* Add an error continuation listener to this `Promise<T>`.
* Once the `Promise<T>` rejects, the [listener] will be called with the error.
*/
fun catch(listener: OnRejectedCallback): Promise<T> {
fun catch(listener: (throwable: Throwable) -> Unit): Promise<T> {
addOnRejectedListener(listener)
return this
}
Expand All @@ -101,18 +89,36 @@ class Promise<T> {
*/
suspend fun await(): T {
return suspendCoroutine { continuation ->
addOnResolvedListener { result -> continuation.resume(result) }
addOnRejectedListener { error -> continuation.resumeWithException(error) }
then { result -> continuation.resume(result) }
catch { error -> continuation.resumeWithException(error) }
}
}

// C++ functions
private external fun nativeResolve(result: Any)
private external fun nativeReject(error: Throwable)
private external fun addOnResolvedListener(callback: OnResolvedCallback<T>)
private external fun addOnResolvedListener(callback: OnResolvedCallback)
private external fun addOnRejectedListener(callback: OnRejectedCallback)
private external fun initHybrid(): HybridData

// Nested callbacks - need to be JavaClasses so we can access them with JNI
@Keep
@DoNotStrip
private fun interface OnResolvedCallback {
@Suppress("unused")
@Keep
@DoNotStrip
fun onResolved(result: Any)
}
@Keep
@DoNotStrip
private fun interface OnRejectedCallback {
@Suppress("unused")
@Keep
@DoNotStrip
fun onRejected(error: Throwable)
}

companion object {
private val defaultScope = CoroutineScope(Dispatchers.Default)

Expand Down
11 changes: 10 additions & 1 deletion packages/template/android/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,18 @@ android {

externalNativeBuild {
cmake {
cppFlags "-O2 -frtti -fexceptions -Wall -fstack-protector-all"
cppFlags "-frtti -fexceptions -Wall -Wextra -fstack-protector-all"
arguments "-DANDROID_STL=c++_shared"
abiFilters (*reactNativeArchitectures())

buildTypes {
debug {
cppFlags "-O1 -g"
}
release {
cppFlags "-O2"
}
}
}
}
}
Expand Down