Internal tracking: b/323342714

There are potential race conditions in ObjectValue.

Past work in response to NullPointerException made access to internal collection more thread safe, but protection is not complete. https://github.com/firebase/firebase-android-sdk/pull/3062

Recent support issue has made me suspicious whether thread safety is at fault. https://github.com/firebase/firebase-android-sdk/issues/5683

This PR should make thread safety complete. In addition, memoization of proto is improved, thereby reducing contention.

Feb 08 '24 16:02 tom-andersen

📝 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.

Feb 08 '24 16:02 github-actions[bot]

Unit Test Results

  180 files ±0   180 suites ±0 3m 55s :stopwatch: -11s 1 226 tests ±0 1 210 :heavy_check_mark: ±0 16 :zzz: ±0 0 :x: ±0 2 476 runs ±0 2 444 :heavy_check_mark: ±0 32 :zzz: ±0 0 :x: ±0

Results for commit 78d1c1e6. ± Comparison against base commit 6a97c306.

:recycle: This comment has been updated with latest results.

Feb 08 '24 16:02 github-actions[bot]

Coverage Report ¹

Affected Products

firebase-firestore
Overall coverage changed from 44.83% (6a97c30) to 44.88% (eac26e6) by +0.05%.
Filename Base (6a97c30) Merge (eac26e6) Diff
DeleteMutation.java 90.48% 95.24% +4.76%
ObjectValue.java 99.06% 98.41% -0.64%
PatchMutation.java 98.39% 100.00% +1.61%

Filename	Base (6a97c30)	Merge (eac26e6)	Diff
DeleteMutation.java	90.48%	95.24%	+4.76%
ObjectValue.java	99.06%	98.41%	-0.64%
PatchMutation.java	98.39%	100.00%	+1.61%

Test Logs

Base (6a97c30): https://github.com/firebase/firebase-android-sdk/actions/runs/7876854964
Merge (eac26e6): https://github.com/firebase/firebase-android-sdk/actions/runs/7892702308

Feb 08 '24 16:02 google-oss-bot

Size Report ¹

Affected Products

firebase-firestore
Type Base (6a97c30) Merge (eac26e6) Diff
aar 1.41 MB 1.41 MB +297 B (+0.0%)
apk (release) 11.4 MB 11.4 MB +384 B (+0.0%)

Type	Base (6a97c30)	Merge (eac26e6)	Diff
aar	1.41 MB	1.41 MB	+297 B (+0.0%)
apk (release)	11.4 MB	11.4 MB	+384 B (+0.0%)

Test Logs

Base (6a97c30): https://github.com/firebase/firebase-android-sdk/actions/runs/7876854964
Merge (eac26e6): https://github.com/firebase/firebase-android-sdk/actions/runs/7892702308

Feb 08 '24 16:02 google-oss-bot

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 (6a97c30) and the CI merge commit (eac26e6)
Please check below reports for each individual commit to find more details (Perfetto traces, histograms, detailed measurements)
- 6a97c30: https://storage.googleapis.com/firebase-sdk-metric-reports/PYEoLZ9PU2/index.html
- eac26e6: https://storage.googleapis.com/firebase-sdk-metric-reports/gEWFAtsbg8/index.html

Startup Times

`fire-fst`

Device

Statistics

Distributions

oriole-32

Percentile	6a97c30	eac26e6	Diff	Significant (?)
p10	418 ±204 μs	562 ±251 μs	+145 μs (+34.7%)	NO
p25	435 ±206 μs	580 ±253 μs	+145 μs (+33.4%)	NO
p50	476 ±206 μs	615 ±244 μs	+139 μs (+29.3%)	NO
p75	536 ±206 μs	662 ±234 μs	+126 μs (+23.5%)	NO
p90	645 ±249 μs	763 ±230 μs	+117 μs (+18.2%)	NO

20 test runs in comparison

Commit	Test Runs
6a97c30	2024-02-12_19:35:34.996370_ggkf 2024-02-12_19:35:34.996415_MKmp 2024-02-12_19:35:34.996427_Dicg 2024-02-12_19:35:34.996447_sRGe 2024-02-12_19:35:34.996458_PNQL 2024-02-12_19:35:34.996466_mvig 2024-02-12_19:35:34.996473_tJnr 2024-02-12_19:35:34.996479_olLr 2024-02-12_19:35:34.996486_qGNa 2024-02-12_19:35:34.996493_vamR
eac26e6	2024-02-13_21:16:01.811143_UDNT 2024-02-13_21:16:01.811179_udeF 2024-02-13_21:16:01.811190_elgP 2024-02-13_21:16:01.811205_MSeF 2024-02-13_21:16:01.811215_cWoy 2024-02-13_21:16:01.811223_TUbd 2024-02-13_21:16:01.811228_AhRU 2024-02-13_21:16:01.811232_VWNa 2024-02-13_21:16:01.811236_kEwX 2024-02-13_21:16:01.811240_VfVW

redfin-30

Percentile	6a97c30	eac26e6	Diff	Significant (?)
p10	608 ±20 μs	683 ±155 μs	+75.0 μs (+12.3%)	NO
p25	630 ±23 μs	705 ±152 μs	+75.7 μs (+12.0%)	NO
p50	668 ±33 μs	737 ±152 μs	+68.5 μs (+10.2%)	NO
p75	713 ±43 μs	791 ±150 μs	+78.4 μs (+11.0%)	NO
p90	777 ±70 μs	956 ±239 μs	+179 μs (+23.0%)	NO

20 test runs in comparison

Commit	Test Runs
6a97c30	2024-02-12_19:35:34.996370_ggkf 2024-02-12_19:35:34.996415_MKmp 2024-02-12_19:35:34.996427_Dicg 2024-02-12_19:35:34.996447_sRGe 2024-02-12_19:35:34.996458_PNQL 2024-02-12_19:35:34.996466_mvig 2024-02-12_19:35:34.996473_tJnr 2024-02-12_19:35:34.996479_olLr 2024-02-12_19:35:34.996486_qGNa 2024-02-12_19:35:34.996493_vamR
eac26e6	2024-02-13_21:16:01.811143_UDNT 2024-02-13_21:16:01.811179_udeF 2024-02-13_21:16:01.811190_elgP 2024-02-13_21:16:01.811205_MSeF 2024-02-13_21:16:01.811215_cWoy 2024-02-13_21:16:01.811223_TUbd 2024-02-13_21:16:01.811228_AhRU 2024-02-13_21:16:01.811232_VWNa 2024-02-13_21:16:01.811236_kEwX 2024-02-13_21:16:01.811240_VfVW

`timeToInitialDisplay`

Device

Statistics

Distributions

oriole-32

Percentile	6a97c30	eac26e6	Diff	Significant (?)
p10	203 ±3 ms	208 ±3 ms	+4.95 ms (+2.4%)	NO
p25	208 ±3 ms	214 ±3 ms	+6.07 ms (+2.9%)	NO
p50	216 ±3 ms	222 ±5 ms	+6.36 ms (+2.9%)	NO
p75	224 ±3 ms	232 ±6 ms	+8.30 ms (+3.7%)	NO
p90	233 ±4 ms	245 ±8 ms	+11.8 ms (+5.1%)	NO

20 test runs in comparison

Commit	Test Runs
6a97c30	2024-02-12_19:35:34.996370_ggkf 2024-02-12_19:35:34.996415_MKmp 2024-02-12_19:35:34.996427_Dicg 2024-02-12_19:35:34.996447_sRGe 2024-02-12_19:35:34.996458_PNQL 2024-02-12_19:35:34.996466_mvig 2024-02-12_19:35:34.996473_tJnr 2024-02-12_19:35:34.996479_olLr 2024-02-12_19:35:34.996486_qGNa 2024-02-12_19:35:34.996493_vamR
eac26e6	2024-02-13_21:16:01.811143_UDNT 2024-02-13_21:16:01.811179_udeF 2024-02-13_21:16:01.811190_elgP 2024-02-13_21:16:01.811205_MSeF 2024-02-13_21:16:01.811215_cWoy 2024-02-13_21:16:01.811223_TUbd 2024-02-13_21:16:01.811228_AhRU 2024-02-13_21:16:01.811232_VWNa 2024-02-13_21:16:01.811236_kEwX 2024-02-13_21:16:01.811240_VfVW

redfin-30

Percentile	6a97c30	eac26e6	Diff	Significant (?)
p10	247 ±3 ms	273 ±6 ms	+26.0 ms (+10.5%)	YES
p25	253 ±3 ms	280 ±6 ms	+26.5 ms (+10.5%)	MAYBE
p50	261 ±3 ms	288 ±7 ms	+27.3 ms (+10.4%)	MAYBE
p75	269 ±3 ms	299 ±9 ms	+29.8 ms (+11.1%)	MAYBE
p90	279 ±4 ms	318 ±14 ms	+38.8 ms (+13.9%)	MAYBE

20 test runs in comparison

Commit	Test Runs
6a97c30	2024-02-12_19:35:34.996370_ggkf 2024-02-12_19:35:34.996415_MKmp 2024-02-12_19:35:34.996427_Dicg 2024-02-12_19:35:34.996447_sRGe 2024-02-12_19:35:34.996458_PNQL 2024-02-12_19:35:34.996466_mvig 2024-02-12_19:35:34.996473_tJnr 2024-02-12_19:35:34.996479_olLr 2024-02-12_19:35:34.996486_qGNa 2024-02-12_19:35:34.996493_vamR
eac26e6	2024-02-13_21:16:01.811143_UDNT 2024-02-13_21:16:01.811179_udeF 2024-02-13_21:16:01.811190_elgP 2024-02-13_21:16:01.811205_MSeF 2024-02-13_21:16:01.811215_cWoy 2024-02-13_21:16:01.811223_TUbd 2024-02-13_21:16:01.811228_AhRU 2024-02-13_21:16:01.811232_VWNa 2024-02-13_21:16:01.811236_kEwX 2024-02-13_21:16:01.811240_VfVW

Feb 08 '24 17:02 google-oss-bot

fix: Make ObjectValue thread safe.

📝 PRs merging into main branch

Unit Test Results

Coverage Report ¹

Affected Products

`firebase-firestore`

Test Logs

Size Report ¹

Affected Products

`firebase-firestore`

Test Logs

Startup Time Report ¹

Notes

Startup Times

`fire-fst`

`timeToInitialDisplay`

fix: Make ObjectValue thread safe.

📝 PRs merging into main branch

Unit Test Results

Coverage Report 1

Affected Products

firebase-firestore

Test Logs

Size Report 1

Affected Products

firebase-firestore

Test Logs

Startup Time Report 1

Notes

Startup Times

fire-fst

timeToInitialDisplay

Coverage Report ¹

`firebase-firestore`

Size Report ¹

`firebase-firestore`

Startup Time Report ¹

`fire-fst`

`timeToInitialDisplay`