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

* perf: Only enable `-O2` in release, use `-O0` in debug

* Also disable for Image and Template

* Switch flags around

* fix: Disable minification

* fix: Slap a good 'ol `mutex` on `Promise`

* fix: Use `emplace_back` on Android

* feat: Add `JUnit` to pass instead of nullptr

* fix: Use `java.lang.Object` for `JUnit`

* fix: Use `java.lang.Object?` for param

* fix: It builds now.

* fix: Fix lambda now

* Restructure

packages/nitrogen/src/syntax/kotlin/KotlinCxxBridgedType.ts

@@ -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({
@@ -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);

packages/react-native-nitro-image/android/build.gradle

@@ -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"
+          }
+        }
       }
     }
   }

packages/react-native-nitro-image/nitrogen/generated/android/c++/JHybridTestObjectSwiftKotlinSpec.cpp

@@ -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"
@@ -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);

packages/react-native-nitro-modules/android/build.gradle

@@ -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"
+          }
+        }
       }
     }
   }

packages/react-native-nitro-modules/android/src/main/cpp/core/JPromise.hpp

@@ -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);
   }
@@ -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);
@@ -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);
@@ -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);
@@ -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); });
     }
   }
 
@@ -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() {

packages/react-native-nitro-modules/android/src/main/cpp/utils/JUnit.hpp

@@ -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

packages/react-native-nitro-modules/android/src/main/java/com/margelo/nitro/core/Promise.kt

@@ -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
@@ -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
@@ -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
   }
@@ -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)
 

packages/template/android/build.gradle

@@ -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"
+          }
+        }
       }
     }
   }