Changing Fireperf's SDK Floats to Doubles #4874

jposuna · 2023-04-07T18:28:25Z

RC Flags are stored as Double and our SDK loses precision when converting back to Float. This causes sampling rates to not behave as expected.

…and our SDK loses precision when converting back to Float. This causes sampling rates to not behave as expected.

github-actions · 2023-04-07T18:28:41Z

📝 PRs merging into main branch

Our main branch should always be in a releasable state. If you are working on a larger change, or if you don't want this change to see the light of the day just yet, consider using a feature branch first, and only merge into the main branch when the code complete and ready to be released.

google-oss-bot · 2023-04-07T18:35:41Z

Coverage Report ¹

Affected Products

firebase-perf
Overall coverage changed from 71.16% (5f273c7) to 71.18% (9a2f9d2) by +0.03%.
Filename Base (5f273c7) Merge (9a2f9d2) Diff
DeviceCacheManager.java 76.03% 76.42% +0.39%

Test Logs

https://storage.googleapis.com/firebase-sdk-metric-reports/OHZqi7bYfk.html

github-actions · 2023-04-07T18:47:51Z

Unit Test Results

  102 files -   647   102 suites - 647 2m 43s ⏱️ - 27m 4s
  975 tests - 3 879   975 ✔️ - 3 856 0 💤 - 21 0 ❌ - 2
1 950 runs - 7 084 1 950 ✔️ - 7 040 0 💤 - 42 0 ❌ - 2

Results for commit 351a810. ± Comparison against base commit fe8c9df.

♻️ This comment has been updated with latest results.

google-oss-bot · 2023-04-07T18:57:15Z

Size Report ¹

Affected Products

firebase-perf
Type Base (5f273c7) Merge (9a2f9d2) Diff
aar 315 kB 315 kB +261 B (+0.1%)
apk (aggressive) 957 kB 957 kB -4 B (-0.0%)
apk (release) 2.95 MB 2.95 MB +252 B (+0.0%)

Test Logs

https://storage.googleapis.com/firebase-sdk-metric-reports/mT0l1dam2a.html

google-oss-bot · 2023-04-07T19:13:37Z

Startup Time Report ¹

Note: Layout is sometimes suboptimal due to limited formatting support on GitHub. Please check this report on GCS.

Notes

This report is for comparing the base commit (5f273c7) and the CI merge commit (9a2f9d2)
Please check below reports for each individual commit to find more details (Perfetto traces, histograms, detailed measurements)
- 5f273c7: https://storage.googleapis.com/firebase-sdk-metric-reports/BKdAfyuVvh/index.html
- 9a2f9d2: https://storage.googleapis.com/firebase-sdk-metric-reports/H4JgleoG1Q/index.html

Startup Times

`fire-perf`

Device Statistics Distributions

oriole-32

Percentile	`5f273c7`	`9a2f9d2`	Diff	Significant (?)
p10	264 ±28 μs	262 ±26 μs	-1.67 μs (-0.6%)	NO
p25	277 ±31 μs	273 ±23 μs	-4.46 μs (-1.6%)	NO
p50	297 ±32 μs	293 ±20 μs	-3.79 μs (-1.3%)	NO
p75	337 ±35 μs	328 ±17 μs	-8.78 μs (-2.6%)	NO
p90	393 ±35 μs	376 ±24 μs	-17.3 μs (-4.4%)	NO

20 test runs in comparison

Commit	Test Runs
`5f273c7`	2023-04-11_00:06:32.092533_BcBm 2023-04-11_00:06:32.098995_kPkJ 2023-04-11_00:06:32.099015_aawx 2023-04-11_00:06:32.099022_GduQ 2023-04-11_00:06:32.099029_qyuQ 2023-04-11_00:06:32.099388_elcQ 2023-04-11_00:06:32.099397_hkRW 2023-04-11_00:06:32.099404_bAPF 2023-04-11_00:06:32.099411_HWDp 2023-04-11_00:06:32.099417_MfLh
`9a2f9d2`	2023-04-11_17:43:02.584131_ZGhR 2023-04-11_17:43:02.585279_fETO 2023-04-11_17:43:02.585287_wvAm 2023-04-11_17:43:02.585293_oSbc 2023-04-11_17:43:02.585299_dGMy 2023-04-11_17:43:02.585309_OKvA 2023-04-11_17:43:02.585315_sMyg 2023-04-11_17:43:02.585320_PtEW 2023-04-11_17:43:02.585325_pOiu 2023-04-11_17:43:02.585331_wsvX

redfin-30

Percentile	`5f273c7`	`9a2f9d2`	Diff	Significant (?)
p10	800 ±65 μs	811 ±72 μs	+11.1 μs (+1.4%)	NO
p25	835 ±59 μs	853 ±73 μs	+17.6 μs (+2.1%)	NO
p50	888 ±67 μs	916 ±73 μs	+27.5 μs (+3.1%)	NO
p75	982 ±102 μs	1.00 ±0.08 ms	+20.1 μs (+2.1%)	NO
p90	1.14 ±0.2 ms	1.19 ±0.2 ms	+42.6 μs (+3.7%)	NO

20 test runs in comparison

Commit	Test Runs
`5f273c7`	2023-04-11_00:06:32.092533_BcBm 2023-04-11_00:06:32.098995_kPkJ 2023-04-11_00:06:32.099015_aawx 2023-04-11_00:06:32.099022_GduQ 2023-04-11_00:06:32.099029_qyuQ 2023-04-11_00:06:32.099388_elcQ 2023-04-11_00:06:32.099397_hkRW 2023-04-11_00:06:32.099404_bAPF 2023-04-11_00:06:32.099411_HWDp 2023-04-11_00:06:32.099417_MfLh
`9a2f9d2`	2023-04-11_17:43:02.584131_ZGhR 2023-04-11_17:43:02.585279_fETO 2023-04-11_17:43:02.585287_wvAm 2023-04-11_17:43:02.585293_oSbc 2023-04-11_17:43:02.585299_dGMy 2023-04-11_17:43:02.585309_OKvA 2023-04-11_17:43:02.585315_sMyg 2023-04-11_17:43:02.585320_PtEW 2023-04-11_17:43:02.585325_pOiu 2023-04-11_17:43:02.585331_wsvX

`timeToInitialDisplay`

Device Statistics Distributions

oriole-32

Percentile	`5f273c7`	`9a2f9d2`	Diff	Significant (?)
p10	193 ±7 ms	196 ±4 ms	+2.94 ms (+1.5%)	NO
p25	199 ±8 ms	202 ±5 ms	+2.86 ms (+1.4%)	NO
p50	206 ±8 ms	210 ±6 ms	+3.52 ms (+1.7%)	NO
p75	215 ±11 ms	219 ±6 ms	+3.53 ms (+1.6%)	NO
p90	225 ±17 ms	232 ±8 ms	+7.07 ms (+3.1%)	NO

20 test runs in comparison

Commit	Test Runs
`5f273c7`	2023-04-11_00:06:32.092533_BcBm 2023-04-11_00:06:32.098995_kPkJ 2023-04-11_00:06:32.099015_aawx 2023-04-11_00:06:32.099022_GduQ 2023-04-11_00:06:32.099029_qyuQ 2023-04-11_00:06:32.099388_elcQ 2023-04-11_00:06:32.099397_hkRW 2023-04-11_00:06:32.099404_bAPF 2023-04-11_00:06:32.099411_HWDp 2023-04-11_00:06:32.099417_MfLh
`9a2f9d2`	2023-04-11_17:43:02.584131_ZGhR 2023-04-11_17:43:02.585279_fETO 2023-04-11_17:43:02.585287_wvAm 2023-04-11_17:43:02.585293_oSbc 2023-04-11_17:43:02.585299_dGMy 2023-04-11_17:43:02.585309_OKvA 2023-04-11_17:43:02.585315_sMyg 2023-04-11_17:43:02.585320_PtEW 2023-04-11_17:43:02.585325_pOiu 2023-04-11_17:43:02.585331_wsvX

redfin-30

Percentile	`5f273c7`	`9a2f9d2`	Diff	Significant (?)
p10	232 ±4 ms	254 ±4 ms	+22.6 ms (+9.8%)	YES
p25	237 ±4 ms	261 ±4 ms	+23.3 ms (+9.8%)	YES
p50	244 ±4 ms	269 ±4 ms	+24.9 ms (+10.2%)	YES
p75	252 ±4 ms	277 ±5 ms	+25.3 ms (+10.0%)	YES
p90	261 ±6 ms	290 ±7 ms	+29.5 ms (+11.3%)	MAYBE

20 test runs in comparison

Commit	Test Runs
`5f273c7`	2023-04-11_00:06:32.092533_BcBm 2023-04-11_00:06:32.098995_kPkJ 2023-04-11_00:06:32.099015_aawx 2023-04-11_00:06:32.099022_GduQ 2023-04-11_00:06:32.099029_qyuQ 2023-04-11_00:06:32.099388_elcQ 2023-04-11_00:06:32.099397_hkRW 2023-04-11_00:06:32.099404_bAPF 2023-04-11_00:06:32.099411_HWDp 2023-04-11_00:06:32.099417_MfLh
`9a2f9d2`	2023-04-11_17:43:02.584131_ZGhR 2023-04-11_17:43:02.585279_fETO 2023-04-11_17:43:02.585287_wvAm 2023-04-11_17:43:02.585293_oSbc 2023-04-11_17:43:02.585299_dGMy 2023-04-11_17:43:02.585309_OKvA 2023-04-11_17:43:02.585315_sMyg 2023-04-11_17:43:02.585320_PtEW 2023-04-11_17:43:02.585325_pOiu 2023-04-11_17:43:02.585331_wsvX

https://storage.googleapis.com/firebase-sdk-metric-reports/ODL8yCFnjc/index.html

visumickey

The changes look good to me.

There is one scenario I'm curious about for testing - Is there a test we can write for one flag that is already cached as Float and how with this new code change we are backward compatible?

firebase-perf/src/main/java/com/google/firebase/perf/config/DeviceCacheManager.java

firebase-perf/src/main/java/com/google/firebase/perf/util/ImmutableBundle.java

…fore storing it in the SharedPreferences.

…is not found.

firebase-perf/src/main/java/com/google/firebase/perf/config/DeviceCacheManager.java

firebase-perf/src/test/java/com/google/firebase/perf/config/DeviceCacheManagerTest.java

…as intended.

Changing Fireperf's Floats to Doubles. RC Flags are stored as Double …

33fe911

…and our SDK loses precision when converting back to Float. This causes sampling rates to not behave as expected.

jposuna requested review from raymondlam and visumickey April 7, 2023 18:28

google-oss-bot added the size/XL label Apr 7, 2023

Included the change in the CHANGELOG

102f003

raymondlam approved these changes Apr 7, 2023

View reviewed changes

visumickey approved these changes Apr 10, 2023

View reviewed changes

jposuna added 3 commits April 10, 2023 15:26

Adding an explanation of why we convert the Double into 'longBits' be…

cc7fece

…fore storing it in the SharedPreferences.

Reusing the existing key but checking for a Float in case the Double …

73e0d68

…is not found.

Fixing formatting

80c3540

jposuna requested a review from visumickey April 11, 2023 00:53

visumickey approved these changes Apr 11, 2023

View reviewed changes

firebase-perf/src/main/java/com/google/firebase/perf/config/DeviceCacheManager.java Show resolved Hide resolved

firebase-perf/src/test/java/com/google/firebase/perf/config/DeviceCacheManagerTest.java Show resolved Hide resolved

Adding a test to verify that overwriting a Float with a Double works …

351a810

…as intended.

jposuna merged commit a267b21 into master Apr 11, 2023

jposuna deleted the jposuna-fireperf-float-to-double branch April 11, 2023 19:59

firebase locked and limited conversation to collaborators May 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Changing Fireperf's SDK Floats to Doubles #4874

Changing Fireperf's SDK Floats to Doubles #4874

Uh oh!

jposuna commented Apr 7, 2023

Uh oh!

github-actions bot commented Apr 7, 2023 •

edited

Loading

Uh oh!

google-oss-bot commented Apr 7, 2023 •

edited

Loading

`firebase-perf`

Uh oh!

github-actions bot commented Apr 7, 2023 •

edited

Loading

Uh oh!

google-oss-bot commented Apr 7, 2023 •

edited

Loading

`firebase-perf`

Uh oh!

google-oss-bot commented Apr 7, 2023 •

edited

Loading

`fire-perf`

`timeToInitialDisplay`

Uh oh!

visumickey left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Changing Fireperf's SDK Floats to Doubles #4874

Changing Fireperf's SDK Floats to Doubles #4874

Uh oh!

Conversation

jposuna commented Apr 7, 2023

Uh oh!

github-actions bot commented Apr 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📝 PRs merging into main branch

Uh oh!

google-oss-bot commented Apr 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report 1

Affected Products

firebase-perf

Test Logs

Uh oh!

github-actions bot commented Apr 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unit Test Results

Uh oh!

google-oss-bot commented Apr 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Size Report 1

Affected Products

firebase-perf

Test Logs

Uh oh!

google-oss-bot commented Apr 7, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Startup Time Report 1

Notes

Startup Times

fire-perf

timeToInitialDisplay

Uh oh!

visumickey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

github-actions bot commented Apr 7, 2023 •

edited

Loading

google-oss-bot commented Apr 7, 2023 •

edited

Loading

Coverage Report ¹

`firebase-perf`

github-actions bot commented Apr 7, 2023 •

edited

Loading

google-oss-bot commented Apr 7, 2023 •

edited

Loading

Size Report ¹

`firebase-perf`

google-oss-bot commented Apr 7, 2023 •

edited

Loading

Startup Time Report ¹

`fire-perf`

`timeToInitialDisplay`