Skip to content

Add FirestoreClientProvider #6050

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 66 commits into from
Aug 12, 2024
Merged

Conversation

tom-andersen
Copy link
Contributor

@tom-andersen tom-andersen commented Jun 19, 2024

Introduce FirestoreClientProvider to manage life cycle and access to FirestoreClient. Calls to FirestoreClient are made through the reference held in FirestoreClientProvider, thereby preparing for swapping FirestoreClient in future PR. This requires rewiring calls, such that code does not hold a direct reference to FirestoreClient.

I am open to a better name than FirestoeClientProvider...

…actor' into tomandersen/componentProviderRefactor
@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jul 4, 2024

Startup Time Report 1

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

Notes

Startup Times

  • fire-fst

    DeviceStatisticsDistributions
    oriole-32
    Percentile57aaf3968b7beaDiffSignificant (?)
    p10461 ±226 μs312 ±21 μs-150 μs (-32.5%)NO
    p25479 ±232 μs322 ±27 μs-157 μs (-32.7%)NO
    p50519 ±232 μs345 ±41 μs-175 μs (-33.6%)NO
    p75611 ±253 μs391 ±76 μs-221 μs (-36.1%)NO
    p90718 ±270 μs497 ±148 μs-221 μs (-30.8%)NO

    20 test runs in comparison
    CommitTest Runs
    57aaf39
    • 2024-08-08_03:04:49.167217_iUlu
    • 2024-08-08_03:04:49.167274_CDeB
    • 2024-08-08_03:04:49.167286_PHKB
    • 2024-08-08_03:04:49.167295_bXta
    • 2024-08-08_03:04:49.167303_RuUt
    • 2024-08-08_03:04:49.167310_chhd
    • 2024-08-08_03:04:49.167318_CsSw
    • 2024-08-08_03:04:49.167325_EoPm
    • 2024-08-08_03:04:49.167332_ngfa
    • 2024-08-08_03:04:49.167343_naGF
    68b7bea
    • 2024-08-12_14:56:20.932760_RPOO
    • 2024-08-12_14:56:20.932803_CDmO
    • 2024-08-12_14:56:20.932813_Poef
    • 2024-08-12_14:56:20.932821_tQba
    • 2024-08-12_14:56:20.932834_FYxG
    • 2024-08-12_14:56:20.932842_Gnpv
    • 2024-08-12_14:56:20.932848_cHQk
    • 2024-08-12_14:56:20.932855_AXDn
    • 2024-08-12_14:56:20.932862_Nhqs
    • 2024-08-12_14:56:20.932869_wFfO
    redfin-30
    Percentile57aaf3968b7beaDiffSignificant (?)
    p10674 ±188 μs630 ±54 μs-44.2 μs (-6.5%)NO
    p25742 ±236 μs656 ±66 μs-86.1 μs (-11.6%)NO
    p50790 ±252 μs692 ±75 μs-97.8 μs (-12.4%)NO
    p75887 ±382 μs750 ±90 μs-137 μs (-15.5%)NO
    p901.02 ±0.4 ms927 ±254 μs-88.7 μs (-8.7%)NO

    20 test runs in comparison
    CommitTest Runs
    57aaf39
    • 2024-08-08_03:04:49.167217_iUlu
    • 2024-08-08_03:04:49.167274_CDeB
    • 2024-08-08_03:04:49.167286_PHKB
    • 2024-08-08_03:04:49.167295_bXta
    • 2024-08-08_03:04:49.167303_RuUt
    • 2024-08-08_03:04:49.167310_chhd
    • 2024-08-08_03:04:49.167318_CsSw
    • 2024-08-08_03:04:49.167325_EoPm
    • 2024-08-08_03:04:49.167332_ngfa
    • 2024-08-08_03:04:49.167343_naGF
    68b7bea
    • 2024-08-12_14:56:20.932760_RPOO
    • 2024-08-12_14:56:20.932803_CDmO
    • 2024-08-12_14:56:20.932813_Poef
    • 2024-08-12_14:56:20.932821_tQba
    • 2024-08-12_14:56:20.932834_FYxG
    • 2024-08-12_14:56:20.932842_Gnpv
    • 2024-08-12_14:56:20.932848_cHQk
    • 2024-08-12_14:56:20.932855_AXDn
    • 2024-08-12_14:56:20.932862_Nhqs
    • 2024-08-12_14:56:20.932869_wFfO
  • timeToInitialDisplay

    DeviceStatisticsDistributions
    oriole-32
    Percentile57aaf3968b7beaDiffSignificant (?)
    p10197 ±3 ms204 ±2 ms+7.33 ms (+3.7%)NO
    p25202 ±3 ms210 ±2 ms+7.77 ms (+3.8%)NO
    p50209 ±4 ms218 ±3 ms+8.48 ms (+4.1%)NO
    p75218 ±5 ms227 ±4 ms+9.52 ms (+4.4%)NO
    p90226 ±5 ms241 ±5 ms+14.4 ms (+6.4%)NO

    20 test runs in comparison
    CommitTest Runs
    57aaf39
    • 2024-08-08_03:04:49.167217_iUlu
    • 2024-08-08_03:04:49.167274_CDeB
    • 2024-08-08_03:04:49.167286_PHKB
    • 2024-08-08_03:04:49.167295_bXta
    • 2024-08-08_03:04:49.167303_RuUt
    • 2024-08-08_03:04:49.167310_chhd
    • 2024-08-08_03:04:49.167318_CsSw
    • 2024-08-08_03:04:49.167325_EoPm
    • 2024-08-08_03:04:49.167332_ngfa
    • 2024-08-08_03:04:49.167343_naGF
    68b7bea
    • 2024-08-12_14:56:20.932760_RPOO
    • 2024-08-12_14:56:20.932803_CDmO
    • 2024-08-12_14:56:20.932813_Poef
    • 2024-08-12_14:56:20.932821_tQba
    • 2024-08-12_14:56:20.932834_FYxG
    • 2024-08-12_14:56:20.932842_Gnpv
    • 2024-08-12_14:56:20.932848_cHQk
    • 2024-08-12_14:56:20.932855_AXDn
    • 2024-08-12_14:56:20.932862_Nhqs
    • 2024-08-12_14:56:20.932869_wFfO
    redfin-30
    Percentile57aaf3968b7beaDiffSignificant (?)
    p10234 ±4 ms275 ±17 ms+40.5 ms (+17.3%)NO
    p25239 ±5 ms285 ±31 ms+45.4 ms (+19.0%)NO
    p50247 ±4 ms294 ±35 ms+47.6 ms (+19.3%)NO
    p75255 ±5 ms305 ±38 ms+50.8 ms (+20.0%)NO
    p90264 ±5 ms326 ±41 ms+61.9 ms (+23.4%)NO

    20 test runs in comparison
    CommitTest Runs
    57aaf39
    • 2024-08-08_03:04:49.167217_iUlu
    • 2024-08-08_03:04:49.167274_CDeB
    • 2024-08-08_03:04:49.167286_PHKB
    • 2024-08-08_03:04:49.167295_bXta
    • 2024-08-08_03:04:49.167303_RuUt
    • 2024-08-08_03:04:49.167310_chhd
    • 2024-08-08_03:04:49.167318_CsSw
    • 2024-08-08_03:04:49.167325_EoPm
    • 2024-08-08_03:04:49.167332_ngfa
    • 2024-08-08_03:04:49.167343_naGF
    68b7bea
    • 2024-08-12_14:56:20.932760_RPOO
    • 2024-08-12_14:56:20.932803_CDmO
    • 2024-08-12_14:56:20.932813_Poef
    • 2024-08-12_14:56:20.932821_tQba
    • 2024-08-12_14:56:20.932834_FYxG
    • 2024-08-12_14:56:20.932842_Gnpv
    • 2024-08-12_14:56:20.932848_cHQk
    • 2024-08-12_14:56:20.932855_AXDn
    • 2024-08-12_14:56:20.932862_Nhqs
    • 2024-08-12_14:56:20.932869_wFfO

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

…ClientProvider

# Conflicts:
#	firebase-firestore/src/main/java/com/google/firebase/firestore/core/ComponentProvider.java
#	firebase-firestore/src/main/java/com/google/firebase/firestore/core/MemoryComponentProvider.java
#	firebase-firestore/src/main/java/com/google/firebase/firestore/core/SQLiteComponentProvider.java
#	firebase-firestore/src/test/java/com/google/firebase/firestore/FirebaseFirestoreIntegrationTestFactory.java
Copy link
Contributor

@wu-hui wu-hui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The overall approach looks good, only some minor nits.

@wu-hui wu-hui assigned tom-andersen and unassigned wu-hui Jul 19, 2024
Copy link
Contributor

@wu-hui wu-hui left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, there are some tests failures though?

@wu-hui wu-hui removed their assignment Aug 2, 2024
@tom-andersen tom-andersen merged commit 08deb69 into main Aug 12, 2024
32 checks passed
@tom-andersen tom-andersen deleted the tomandersen/firestoreClientProvider branch August 12, 2024 16:34
@firebase firebase locked and limited conversation to collaborators Sep 13, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants