perf: Move updateNative(...) into a protected override to seal it (#…

…499) * chore: Move `updateNative(...)` into a protected override to seal it * chore: Adjust comments on updateNative * fix: Remove HybridData in `HybridView` * remove unused import
mrousavy · Jan 15, 2025 · d4cfc2b · d4cfc2b
1 parent 3387184
commit d4cfc2b
Show file tree

Hide file tree

Showing 10 changed files with 8 additions and 90 deletions.
diff --git a/packages/nitrogen/src/syntax/kotlin/KotlinHybridObject.ts b/packages/nitrogen/src/syntax/kotlin/KotlinHybridObject.ts
@@ -45,7 +45,6 @@ ${createFileMetadataString(`${name.HybridTSpec}.kt`)}
 
 package ${javaPackage}
 
-import android.util.Log
 import androidx.annotation.Keep
 import com.facebook.jni.HybridData
 import com.facebook.proguard.annotations.DoNotStrip
@@ -67,14 +66,9 @@ abstract class ${name.HybridTSpec}: ${kotlinBase}() {
   private var mHybridData: HybridData = initHybrid()
 
   init {
-    // Pass this \`HybridData\` through to it's base class,
-    // to represent inheritance to JHybridObject on C++ side
     super.updateNative(mHybridData)
   }
 
-  /**
-   * Call from a child class to initialize HybridData with a child.
-   */
   override fun updateNative(hybridData: HybridData) {
     mHybridData = hybridData
     super.updateNative(hybridData)

diff --git a/...e-nitro-image/nitrogen/generated/android/kotlin/com/margelo/nitro/image/HybridBaseSpec.kt b/...e-nitro-image/nitrogen/generated/android/kotlin/com/margelo/nitro/image/HybridBaseSpec.kt
@@ -7,7 +7,6 @@
 
 package com.margelo.nitro.image
 
-import android.util.Log
 import androidx.annotation.Keep
 import com.facebook.jni.HybridData
 import com.facebook.proguard.annotations.DoNotStrip
@@ -29,14 +28,9 @@ abstract class HybridBaseSpec: HybridObject() {
   private var mHybridData: HybridData = initHybrid()
 
   init {
-    // Pass this `HybridData` through to it's base class,
-    // to represent inheritance to JHybridObject on C++ side
     super.updateNative(mHybridData)
   }
 
-  /**
-   * Call from a child class to initialize HybridData with a child.
-   */
   override fun updateNative(hybridData: HybridData) {
     mHybridData = hybridData
     super.updateNative(hybridData)

diff --git a/...-nitro-image/nitrogen/generated/android/kotlin/com/margelo/nitro/image/HybridChildSpec.kt b/...-nitro-image/nitrogen/generated/android/kotlin/com/margelo/nitro/image/HybridChildSpec.kt
@@ -7,7 +7,6 @@
 
 package com.margelo.nitro.image
 
-import android.util.Log
 import androidx.annotation.Keep
 import com.facebook.jni.HybridData
 import com.facebook.proguard.annotations.DoNotStrip
@@ -29,14 +28,9 @@ abstract class HybridChildSpec: HybridBaseSpec() {
   private var mHybridData: HybridData = initHybrid()
 
   init {
-    // Pass this `HybridData` through to it's base class,
-    // to represent inheritance to JHybridObject on C++ side
     super.updateNative(mHybridData)
   }
 
-  /**
-   * Call from a child class to initialize HybridData with a child.
-   */
   override fun updateNative(hybridData: HybridData) {
     mHybridData = hybridData
     super.updateNative(hybridData)

diff --git a/...image/nitrogen/generated/android/kotlin/com/margelo/nitro/image/HybridImageFactorySpec.kt b/...image/nitrogen/generated/android/kotlin/com/margelo/nitro/image/HybridImageFactorySpec.kt
@@ -7,7 +7,6 @@
 
 package com.margelo.nitro.image
 
-import android.util.Log
 import androidx.annotation.Keep
 import com.facebook.jni.HybridData
 import com.facebook.proguard.annotations.DoNotStrip
@@ -29,14 +28,9 @@ abstract class HybridImageFactorySpec: HybridObject() {
   private var mHybridData: HybridData = initHybrid()
 
   init {
-    // Pass this `HybridData` through to it's base class,
-    // to represent inheritance to JHybridObject on C++ side
     super.updateNative(mHybridData)
   }
 
-  /**
-   * Call from a child class to initialize HybridData with a child.
-   */
   override fun updateNative(hybridData: HybridData) {
     mHybridData = hybridData
     super.updateNative(hybridData)

diff --git a/...-nitro-image/nitrogen/generated/android/kotlin/com/margelo/nitro/image/HybridImageSpec.kt b/...-nitro-image/nitrogen/generated/android/kotlin/com/margelo/nitro/image/HybridImageSpec.kt
@@ -7,7 +7,6 @@
 
 package com.margelo.nitro.image
 
-import android.util.Log
 import androidx.annotation.Keep
 import com.facebook.jni.HybridData
 import com.facebook.proguard.annotations.DoNotStrip
@@ -29,14 +28,9 @@ abstract class HybridImageSpec: HybridObject() {
   private var mHybridData: HybridData = initHybrid()
 
   init {
-    // Pass this `HybridData` through to it's base class,
-    // to represent inheritance to JHybridObject on C++ side
     super.updateNative(mHybridData)
   }
 
-  /**
-   * Call from a child class to initialize HybridData with a child.
-   */
   override fun updateNative(hybridData: HybridData) {
     mHybridData = hybridData
     super.updateNative(hybridData)

diff --git a/...rogen/generated/android/kotlin/com/margelo/nitro/image/HybridTestObjectSwiftKotlinSpec.kt b/...rogen/generated/android/kotlin/com/margelo/nitro/image/HybridTestObjectSwiftKotlinSpec.kt
@@ -7,7 +7,6 @@
 
 package com.margelo.nitro.image
 
-import android.util.Log
 import androidx.annotation.Keep
 import com.facebook.jni.HybridData
 import com.facebook.proguard.annotations.DoNotStrip
@@ -29,14 +28,9 @@ abstract class HybridTestObjectSwiftKotlinSpec: HybridObject() {
   private var mHybridData: HybridData = initHybrid()
 
   init {
-    // Pass this `HybridData` through to it's base class,
-    // to represent inheritance to JHybridObject on C++ side
     super.updateNative(mHybridData)
   }
 
-  /**
-   * Call from a child class to initialize HybridData with a child.
-   */
   override fun updateNative(hybridData: HybridData) {
     mHybridData = hybridData
     super.updateNative(hybridData)

diff --git a/...tro-image/nitrogen/generated/android/kotlin/com/margelo/nitro/image/HybridTestViewSpec.kt b/...tro-image/nitrogen/generated/android/kotlin/com/margelo/nitro/image/HybridTestViewSpec.kt
@@ -7,7 +7,6 @@
 
 package com.margelo.nitro.image
 
-import android.util.Log
 import androidx.annotation.Keep
 import com.facebook.jni.HybridData
 import com.facebook.proguard.annotations.DoNotStrip
@@ -30,14 +29,9 @@ abstract class HybridTestViewSpec: HybridView() {
   private var mHybridData: HybridData = initHybrid()
 
   init {
-    // Pass this `HybridData` through to it's base class,
-    // to represent inheritance to JHybridObject on C++ side
     super.updateNative(mHybridData)
   }
 
-  /**
-   * Call from a child class to initialize HybridData with a child.
-   */
   override fun updateNative(hybridData: HybridData) {
     mHybridData = hybridData
     super.updateNative(hybridData)

diff --git a/...s/react-native-nitro-modules/android/src/main/java/com/margelo/nitro/core/HybridObject.kt b/...s/react-native-nitro-modules/android/src/main/java/com/margelo/nitro/core/HybridObject.kt
@@ -9,7 +9,7 @@ import com.facebook.proguard.annotations.DoNotStrip
  */
 @Keep
 @DoNotStrip
-abstract class HybridObject: ExtendableHybridClass {
+abstract class HybridObject {
     /**
      * Get the memory size of the Kotlin instance (plus any external heap allocations),
      * in bytes.
@@ -39,10 +39,14 @@ abstract class HybridObject: ExtendableHybridClass {
     private var mHybridData: HybridData? = null
 
     /**
-     * Must be called in the constructor of a subclass of `HybridObject`, to initialize the C++
-     * `JHybridObject` with a subclass of it.
+     * If `HybridObject` is subclassed, the sub-class needs to create it's own `HybridData`
+     * with a C++ `jni::HybridClass` representing the subclass directly.
+     * Then, that `HybridData` must be passed upwards to `HybridObject` using `updateNative(..)`.
+     *
+     * This must happen for each sub/base class in the whole inheritance chain to ensure
+     * overrides and type-erasure works as expected.
      */
-    override fun updateNative(hybridData: HybridData) {
+    protected open fun updateNative(hybridData: HybridData) {
         mHybridData = hybridData
     }
 }
diff --git a/...tive-nitro-modules/android/src/main/java/com/margelo/nitro/utils/ExtendableHybridClass.kt b/...tive-nitro-modules/android/src/main/java/com/margelo/nitro/utils/ExtendableHybridClass.kt
diff --git a/...es/react-native-nitro-modules/android/src/main/java/com/margelo/nitro/views/HybridView.kt b/...es/react-native-nitro-modules/android/src/main/java/com/margelo/nitro/views/HybridView.kt
@@ -20,19 +20,4 @@ abstract class HybridView: HybridObject() {
     @get:DoNotStrip
     @get:Keep
     abstract val view: View
-
-    /**
-     * Holds the native C++ instance.
-     * In `HybridView`, the C++ instance is a sub-class of `JHybridView`, such as one of it's specs.
-     * This is `null`, until `updateNative(..)` is called.
-     */
-    private var mHybridData: HybridData? = null
-
-    /**
-     * Must be called in the constructor of a subclass of `HybridObject`, to initialize the C++
-     * `JHybridObject` with a subclass of it.
-     */
-    override fun updateNative(hybridData: HybridData) {
-        mHybridData = hybridData
-    }
 }