fix(firebase-perf): Update device cache only if RC value is different…

… from cached value (#6431) Fixes #6407
firebase · Dec 19, 2024 · 49a637f · 49a637f
1 parent e3dd809
commit 49a637f
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 7 deletions.
diff --git a/firebase-perf/CHANGELOG.md b/firebase-perf/CHANGELOG.md
@@ -1,5 +1,7 @@
 # Unreleased
 
+* [fixed] Fixed a performance issue with shared preferences
+  calling `.apply()` every time a value is read from remote config (#6407)
 
 # 21.0.3
 * [changed] Bump internal dependencies.

diff --git a/firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigResolver.java b/firebase-perf/src/main/java/com/google/firebase/perf/config/ConfigResolver.java
@@ -221,6 +221,7 @@ private boolean getIsSdkEnabled() {
     // 2. If the value exists in device cache, return this value.
     // 3. Otherwise, return default value.
     SdkEnabled config = SdkEnabled.getInstance();
+    Optional<Boolean> deviceCacheValue = getDeviceCacheBoolean(config);
 
     // 1. Reads value from Firebase Remote Config, saves this value in cache layer if fetch status
     // is not failure.
@@ -230,13 +231,19 @@ private boolean getIsSdkEnabled() {
       if (remoteConfigManager.isLastFetchFailed()) {
         return false;
       }
-      // b. Cache and return this value.
-      deviceCacheManager.setValue(config.getDeviceCacheFlag(), rcValue.get());
-      return rcValue.get();
+
+      Boolean newValue = rcValue.get();
+      // b. Only cache and return this value if it is different from the current value.
+      if (deviceCacheValue == null
+          || !deviceCacheValue.isAvailable()
+          || deviceCacheValue.get() != newValue) {
+        deviceCacheManager.setValue(config.getDeviceCacheFlag(), newValue);
+      }
+
+      return newValue;
     }
 
     // 2. If the value exists in device cache, return this value.
-    Optional<Boolean> deviceCacheValue = getDeviceCacheBoolean(config);
     if (deviceCacheValue.isAvailable()) {
       return deviceCacheValue.get();
     }
@@ -257,17 +264,23 @@ private boolean getIsSdkVersionDisabled() {
     // 2. If the value exists in device cache, return this value.
     // 3. Otherwise, return default value.
     SdkDisabledVersions config = SdkDisabledVersions.getInstance();
+    Optional<String> deviceCacheValue = getDeviceCacheString(config);
 
     // 1. Reads value from Firebase Remote Config, cache and return this value.
     Optional<String> rcValue = getRemoteConfigString(config);
     if (rcValue.isAvailable()) {
       // Do not check FRC last fetch status because it is the most recent value device can get.
-      deviceCacheManager.setValue(config.getDeviceCacheFlag(), rcValue.get());
-      return isFireperfSdkVersionInList(rcValue.get());
+      String newValue = rcValue.get();
+      // Only cache and return this value if it is different from the current value.
+      if (deviceCacheValue == null
+          || !deviceCacheValue.isAvailable()
+          || !deviceCacheValue.get().equals(newValue)) {
+        deviceCacheManager.setValue(config.getDeviceCacheFlag(), newValue);
+      }
+      return isFireperfSdkVersionInList(newValue);
     }
 
     // 2. If the value exists in device cache, return this value.
-    Optional<String> deviceCacheValue = getDeviceCacheString(config);
     if (deviceCacheValue.isAvailable()) {
       return isFireperfSdkVersionInList(deviceCacheValue.get());
     }

diff --git a/firebase-perf/src/test/java/com/google/firebase/perf/config/ConfigResolverTest.java b/firebase-perf/src/test/java/com/google/firebase/perf/config/ConfigResolverTest.java
@@ -280,6 +280,23 @@ public void getIsServiceCollectionEnabled_sdkDisabledVersionFlagNoFrc_returnDefa
     verify(mockDeviceCacheManager, never()).setValue(any(), anyString());
   }
 
+  @Test
+  public void getIsServiceCollectionEnabled_deviceCacheHasSameValueAsFrc_returnCacheValue() {
+    when(mockRemoteConfigManager.getBoolean(FIREBASE_PERFORMANCE_SDK_ENABLED_FRC_KEY))
+        .thenReturn(Optional.of(true));
+    when(mockDeviceCacheManager.getBoolean(FIREBASE_PERFORMANCE_SDK_ENABLED_CACHE_KEY))
+        .thenReturn(Optional.of(true));
+
+    when(mockDeviceCacheManager.getString(FIREBASE_PERFORMANCE_DISABLED_VERSIONS_CACHE_KEY))
+        .thenReturn(Optional.of(""));
+    when(mockRemoteConfigManager.getString(FIREBASE_PERFORMANCE_DISABLED_VERSIONS_FRC_KEY))
+        .thenReturn(Optional.of(""));
+
+    assertThat(testConfigResolver.getIsServiceCollectionEnabled()).isTrue();
+    verify(mockDeviceCacheManager, never()).setValue(any(), anyBoolean());
+    verify(mockDeviceCacheManager, never()).setValue(any(), anyString());
+  }
+
   @Test
   public void
       getIsPerformanceCollectionConfigValueAvailable_noDeviceCacheNoRemoteConfig_returnsFalse() {