From bdce711f4d675b53372d461113490559fd76d76c Mon Sep 17 00:00:00 2001 From: Jonathan Moskovich <48201295+jonathanmos@users.noreply.github.com> Date: Wed, 20 Dec 2023 12:52:04 +0200 Subject: [PATCH] Performance optimizations --- .../DdSessionReplayImplementation.kt | 6 +- ...eactNativeSessionReplayExtensionSupport.kt | 6 +- .../ReactTextPropertiesResolver.kt | 70 ++++++++++++------- .../sessionreplay/SessionReplaySDKWrapper.kt | 9 --- .../sessionreplay/ShadowNodeWrapper.kt | 7 +- .../sessionreplay/utils/ReflectionUtils.kt | 26 +++---- ...NativeSessionReplayExtensionSupportTest.kt | 10 +-- .../ReactTextPropertiesResolverTest.kt | 2 +- 8 files changed, 70 insertions(+), 66 deletions(-) diff --git a/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/DdSessionReplayImplementation.kt b/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/DdSessionReplayImplementation.kt index 3eefe6ffb..fd807598d 100644 --- a/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/DdSessionReplayImplementation.kt +++ b/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/DdSessionReplayImplementation.kt @@ -6,6 +6,8 @@ package com.datadog.reactnative.sessionreplay +import com.datadog.android.Datadog +import com.datadog.android.api.feature.FeatureSdkCore import com.datadog.android.sessionreplay.SessionReplayConfiguration import com.datadog.android.sessionreplay.SessionReplayPrivacy import com.facebook.react.bridge.Promise @@ -27,9 +29,11 @@ class DdSessionReplayImplementation( * @param defaultPrivacyLevel The privacy level used for replay. */ fun enable(replaySampleRate: Double, defaultPrivacyLevel: String, promise: Promise) { + val sdkCore = Datadog.getInstance() as FeatureSdkCore + val logger = sdkCore.internalLogger val configuration = SessionReplayConfiguration.Builder(replaySampleRate.toFloat()) .setPrivacy(buildPrivacy(defaultPrivacyLevel)) - .addExtensionSupport(ReactNativeSessionReplayExtensionSupport(reactContext)) + .addExtensionSupport(ReactNativeSessionReplayExtensionSupport(reactContext, logger)) .build() sessionReplayProvider().enable(configuration) promise.resolve(null) diff --git a/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/ReactNativeSessionReplayExtensionSupport.kt b/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/ReactNativeSessionReplayExtensionSupport.kt index 4f38a7bee..c415e9734 100644 --- a/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/ReactNativeSessionReplayExtensionSupport.kt +++ b/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/ReactNativeSessionReplayExtensionSupport.kt @@ -6,7 +6,6 @@ package com.datadog.reactnative.sessionreplay -import android.annotation.SuppressLint import android.view.View import androidx.annotation.VisibleForTesting import com.datadog.android.api.InternalLogger @@ -24,7 +23,7 @@ import com.facebook.react.views.view.ReactViewGroup internal class ReactNativeSessionReplayExtensionSupport( private val reactContext: ReactContext, - private val logger: InternalLogger? = SessionReplaySDKWrapper.getLogger() + private val logger: InternalLogger ) : ExtensionSupport { override fun getCustomViewMappers(): Map, WireframeMapper>> { @@ -45,13 +44,12 @@ internal class ReactNativeSessionReplayExtensionSupport( return listOf() } - @SuppressLint("LongLogTag") @VisibleForTesting internal fun getUiManagerModule(): UIManagerModule? { return try { reactContext.getNativeModule(UIManagerModule::class.java) } catch (e: IllegalStateException) { - logger?.log( + logger.log( level = InternalLogger.Level.WARN, targets = listOf(InternalLogger.Target.MAINTAINER, InternalLogger.Target.TELEMETRY), messageBuilder = { RESOLVE_UIMANAGERMODULE_ERROR }, diff --git a/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/ReactTextPropertiesResolver.kt b/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/ReactTextPropertiesResolver.kt index 174039169..07fd80020 100644 --- a/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/ReactTextPropertiesResolver.kt +++ b/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/ReactTextPropertiesResolver.kt @@ -34,38 +34,63 @@ internal class ReactTextPropertiesResolver( view: TextView, pixelDensity: Float, ): MobileSegment.Wireframe.TextWireframe { - val textWireframe = resolveDrawableProperties(view, pixelDensity, originalWireframe) + val (shapeStyle, border) = resolveShapeStyleAndBorder(view, pixelDensity) + ?: (originalWireframe.shapeStyle to originalWireframe.border) + + val (textStyle, textPosition) = resolveTextStyleAndPosition( + originalWireframe, + view, + pixelDensity + ) ?: (originalWireframe.textStyle to originalWireframe.textPosition) + + // nothing changed, return the original wireframe + @Suppress("ComplexCondition") + if (shapeStyle == originalWireframe.shapeStyle + && border == originalWireframe.border + && textStyle == originalWireframe.textStyle + && textPosition == originalWireframe.textPosition + ) { + return originalWireframe + } + + return originalWireframe.copy( + shapeStyle = shapeStyle, + border = border, + textStyle = textStyle, + textPosition = textPosition + ) + } + private fun resolveTextStyleAndPosition( + originalWireframe: MobileSegment.Wireframe.TextWireframe, + view: TextView, + pixelDensity: Float, + ): + Pair? { val shadowNodeWrapper: ShadowNodeWrapper = ShadowNodeWrapper.getShadowNodeWrapper( reactContext = reactContext, uiManagerModule = uiManagerModule, reflectionUtils = reflectionUtils, - viewId = view.id) ?: return textWireframe + viewId = view.id) ?: return null - val textStyle = resolveTextStyle(textWireframe, pixelDensity, shadowNodeWrapper) - val textPosition = textWireframe.textPosition - val padding = textPosition?.padding - val alignment = resolveTextAlignment(view, textWireframe) + val textStyle = resolveTextStyle(originalWireframe, pixelDensity, shadowNodeWrapper) + val alignment = resolveTextAlignment(view, originalWireframe) - return textWireframe.copy( - textStyle = textStyle, - textPosition = MobileSegment.TextPosition( - alignment = alignment, - padding = padding - ) + val textPosition = MobileSegment.TextPosition( + alignment = alignment, + padding = originalWireframe.textPosition?.padding ) + + return textStyle to textPosition } - private fun resolveDrawableProperties( + private fun resolveShapeStyleAndBorder( view: TextView, pixelDensity: Float, - textWireframe: MobileSegment.Wireframe.TextWireframe - ): MobileSegment.Wireframe.TextWireframe { + ): Pair? { val backgroundDrawable: ReactViewBackgroundDrawable = - drawableUtils.getReactBackgroundFromDrawable(view.background) ?: return textWireframe - - var resultWireframe = textWireframe + drawableUtils.getReactBackgroundFromDrawable(view.background) ?: return null // view.alpha is the value of the opacity prop on the js side val opacity = view.alpha @@ -74,14 +99,7 @@ internal class ReactTextPropertiesResolver( reactViewBackgroundDrawableUtils .resolveShapeAndBorder(backgroundDrawable, opacity, pixelDensity) - if (shapeStyle != null || border != null) { - resultWireframe = resultWireframe.copy( - shapeStyle = shapeStyle, - border = border - ) - } - - return resultWireframe + return shapeStyle to border } private fun resolveTextAlignment( diff --git a/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/SessionReplaySDKWrapper.kt b/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/SessionReplaySDKWrapper.kt index 06842ddfc..7fa70e152 100644 --- a/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/SessionReplaySDKWrapper.kt +++ b/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/SessionReplaySDKWrapper.kt @@ -6,9 +6,6 @@ package com.datadog.reactnative.sessionreplay -import com.datadog.android.Datadog -import com.datadog.android.api.InternalLogger -import com.datadog.android.api.feature.FeatureSdkCore import com.datadog.android.sessionreplay.SessionReplay import com.datadog.android.sessionreplay.SessionReplayConfiguration @@ -24,10 +21,4 @@ internal class SessionReplaySDKWrapper : SessionReplayWrapper { sessionReplayConfiguration, ) } - - internal companion object { - internal fun getLogger(): InternalLogger? { - return (Datadog.getInstance() as? FeatureSdkCore)?.internalLogger - } - } } diff --git a/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/ShadowNodeWrapper.kt b/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/ShadowNodeWrapper.kt index bf549daa2..98da40a7a 100644 --- a/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/ShadowNodeWrapper.kt +++ b/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/ShadowNodeWrapper.kt @@ -15,6 +15,7 @@ import com.facebook.react.uimanager.ReactShadowNode import com.facebook.react.uimanager.UIImplementation import com.facebook.react.uimanager.UIManagerModule import java.util.concurrent.CountDownLatch +import java.util.concurrent.TimeUnit internal class ShadowNodeWrapper( private val shadowNode: ReactShadowNode>?, @@ -49,7 +50,7 @@ internal class ShadowNodeWrapper( } reactContext.runOnNativeModulesQueueThread(shadowNodeRunnable) - countDownLatch.await() + countDownLatch.await(5, TimeUnit.SECONDS) if (target == null) { return null @@ -60,9 +61,7 @@ internal class ShadowNodeWrapper( private fun resolveShadowNode(reflectionUtils: ReflectionUtils, uiManagerModule: UIManagerModule, tag: Int): ReactShadowNode>? { val uiManagerImplementation = reflectionUtils.getDeclaredField(uiManagerModule, UI_IMPLEMENTATION_FIELD_NAME) as UIImplementation? - ?: return null - - return uiManagerImplementation.resolveShadowNode(tag) + return uiManagerImplementation?.resolveShadowNode(tag) } @VisibleForTesting diff --git a/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/utils/ReflectionUtils.kt b/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/utils/ReflectionUtils.kt index 3e518c165..16cb92819 100644 --- a/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/utils/ReflectionUtils.kt +++ b/packages/react-native-session-replay/android/src/main/kotlin/com/datadog/reactnative/sessionreplay/utils/ReflectionUtils.kt @@ -10,27 +10,21 @@ import java.lang.reflect.Field internal class ReflectionUtils { internal fun getDeclaredField(instance: Any, fieldName: String): Any? { - val className = instance.javaClass - val declaredField = searchForField(className, fieldName) ?: return null + val classInstance = instance.javaClass + val declaredField = searchForField(classInstance, fieldName) - declaredField.let { + return declaredField?.let { it.isAccessible = true - return it.get(instance) + it.get(instance) } } private fun searchForField(className: Class<*>, fieldName: String): Field? { - val isFieldDefinedForClass = className.declaredFields.firstOrNull{ it.name == fieldName } != null - val hasSuperclass = className.superclass != null - - if (isFieldDefinedForClass) { - return className.getDeclaredField(fieldName) - } - - if (hasSuperclass) { - return searchForField(className.superclass, fieldName) - } - - return null + return className.declaredFields.firstOrNull { it.name == fieldName } + ?: if (className.superclass != null) { + searchForField(className.superclass, fieldName) + } else { + null + } } } diff --git a/packages/react-native-session-replay/android/src/test/kotlin/com/datadog/reactnative/sessionreplay/ReactNativeSessionReplayExtensionSupportTest.kt b/packages/react-native-session-replay/android/src/test/kotlin/com/datadog/reactnative/sessionreplay/ReactNativeSessionReplayExtensionSupportTest.kt index d1d23f522..93c931c03 100644 --- a/packages/react-native-session-replay/android/src/test/kotlin/com/datadog/reactnative/sessionreplay/ReactNativeSessionReplayExtensionSupportTest.kt +++ b/packages/react-native-session-replay/android/src/test/kotlin/com/datadog/reactnative/sessionreplay/ReactNativeSessionReplayExtensionSupportTest.kt @@ -10,7 +10,6 @@ import com.datadog.android.api.InternalLogger import com.datadog.android.sessionreplay.SessionReplayPrivacy import com.datadog.reactnative.sessionreplay.mappers.ReactTextMapper import com.datadog.reactnative.sessionreplay.mappers.ReactViewGroupMapper -import com.datadog.tools.unit.GenericAssert.Companion.assertThat import com.facebook.react.bridge.NativeModule import com.facebook.react.bridge.ReactContext import com.facebook.react.uimanager.UIManagerModule @@ -18,6 +17,7 @@ import com.facebook.react.views.text.ReactTextView import com.facebook.react.views.textinput.ReactEditText import com.facebook.react.views.view.ReactViewGroup import fr.xgouchet.elmyr.junit5.ForgeExtension +import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.ExtendWith @@ -67,12 +67,12 @@ internal class ReactNativeSessionReplayExtensionSupportTest { // Then check(allowMappers != null) - assertThat(allowMappers.size).isEqualTo(3) - assertThat(allowMappers.get(ReactViewGroup::class.java)) + assertThat(allowMappers).hasSize(3) + assertThat(allowMappers[ReactViewGroup::class.java]) .isInstanceOf(ReactViewGroupMapper::class.java) - assertThat(allowMappers.get(ReactTextView::class.java)) + assertThat(allowMappers[ReactTextView::class.java]) .isInstanceOf(ReactTextMapper::class.java) - assertThat(allowMappers.get(ReactEditText::class.java)) + assertThat(allowMappers[ReactEditText::class.java]) .isInstanceOf(ReactTextMapper::class.java) } diff --git a/packages/react-native-session-replay/android/src/test/kotlin/com/datadog/reactnative/sessionreplay/ReactTextPropertiesResolverTest.kt b/packages/react-native-session-replay/android/src/test/kotlin/com/datadog/reactnative/sessionreplay/ReactTextPropertiesResolverTest.kt index 11438b17d..66f1e9780 100644 --- a/packages/react-native-session-replay/android/src/test/kotlin/com/datadog/reactnative/sessionreplay/ReactTextPropertiesResolverTest.kt +++ b/packages/react-native-session-replay/android/src/test/kotlin/com/datadog/reactnative/sessionreplay/ReactTextPropertiesResolverTest.kt @@ -18,7 +18,6 @@ import com.datadog.reactnative.sessionreplay.utils.ReactViewBackgroundDrawableUt import com.datadog.reactnative.sessionreplay.utils.ReflectionUtils import com.datadog.reactnative.sessionreplay.utils.formatAsRgba import com.datadog.reactnative.tools.unit.forge.ForgeConfigurator -import com.datadog.tools.unit.GenericAssert.Companion.assertThat import com.facebook.react.bridge.ReactContext import com.facebook.react.uimanager.ReactShadowNode import com.facebook.react.uimanager.UIImplementation @@ -30,6 +29,7 @@ import fr.xgouchet.elmyr.annotation.Forgery import fr.xgouchet.elmyr.annotation.IntForgery import fr.xgouchet.elmyr.junit5.ForgeConfiguration import fr.xgouchet.elmyr.junit5.ForgeExtension +import org.assertj.core.api.Assertions.assertThat import org.junit.jupiter.api.BeforeEach import org.junit.jupiter.api.Test import org.junit.jupiter.api.extension.ExtendWith