Update component runtime to pick up any one of the couroutine dispatcher interface #5300

VinayGuthal · 2023-09-06T15:11:23Z

No description provided.

github-actions · 2023-09-06T15:11:41Z

Release note changes

The following had changelogs that were modified, but did not have any unreleased entries for release notes to generate from.

Changelogs

firebase-components

google-oss-bot · 2023-09-06T15:15:50Z

Coverage Report ¹

Affected Products

firebase-components
Overall coverage changed from 55.34% (fe84885) to 55.74% (6b67ec8) by +0.40%.
Filename Base (fe84885) Merge (6b67ec8) Diff
ComponentRuntime.java 72.32% 71.81% -0.51%
Qualified.java 76.47% 88.24% +11.76%

Test Logs

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

github-actions · 2023-09-06T15:18:13Z

Unit Test Results

  24 files +  18   24 suites +18 16s ⏱️ -2s
115 tests +  97 115 ✔️ +  97 0 💤 ±0 0 ❌ ±0
230 runs +194 230 ✔️ +194 0 💤 ±0 0 ❌ ±0

Results for commit 8a92f99. ± Comparison against base commit fe84885.

This pull request removes 18 and adds 115 tests. Note that renamed tests count towards both.

com.google.firebase.crashlytics.internal.common.CrashlyticsControllerRobolectricTest ‑ testDoCloseSession_disabledAnrs_doesNotPersistsAppExitInfo
com.google.firebase.crashlytics.internal.common.CrashlyticsControllerRobolectricTest ‑ testDoCloseSession_enabledAnrs_doesNotPersistsAppExitInfoIfItDoesntExist
com.google.firebase.crashlytics.internal.common.CrashlyticsControllerRobolectricTest ‑ testDoCloseSession_enabledAnrs_persistsAppExitInfoIfItExists
com.google.firebase.crashlytics.internal.common.SessionReportingCoordinatorRobolectricTest ‑ testAppExitInfoEvent_notPersistIfAnrBeforeSession
com.google.firebase.crashlytics.internal.common.SessionReportingCoordinatorRobolectricTest ‑ testAppExitInfoEvent_notPersistIfAppExitInfoNotAnrButWithinSession
com.google.firebase.crashlytics.internal.common.SessionReportingCoordinatorRobolectricTest ‑ testAppExitInfoEvent_persistIfAnrWithinSession
com.google.firebase.crashlytics.internal.common.SessionReportingCoordinatorRobolectricTest ‑ testAppExitInfoEvent_persistIfAnrWithinSession_multipleAppExitInfo
com.google.firebase.crashlytics.internal.common.SessionReportingCoordinatorRobolectricTest ‑ testconvertInputStreamToString_worksSuccessfully
com.google.firebase.crashlytics.internal.model.CrashlyticsReportTest ‑ testGetBinaryImageUuidUtf8Bytes_returnsNullWhenUuidIsNull
com.google.firebase.crashlytics.internal.model.CrashlyticsReportTest ‑ testGetBinaryImageUuidUtf8Bytes_returnsProperBytes
…

com.google.firebase.components.ComponentDiscoveryTest ‑ discoverLazy_whenRegistrarClassDoesNotExist_shouldReturnProviderThatReturnsNull
com.google.firebase.components.ComponentDiscoveryTest ‑ discoverLazy_whenRegistrarClassesAreInvalid_shouldReturnThrowingProviders
com.google.firebase.components.ComponentDiscoveryTest ‑ discover_shouldCorrectlyInstantiateValidComponentRegistrars
com.google.firebase.components.ComponentDiscoveryTest ‑ discover_shouldSkipClassesThatDontImplementComponentRegistrarInterface
com.google.firebase.components.ComponentDiscoveryTest ‑ discover_shouldSkipClassesWithNoDefaultConstructors
com.google.firebase.components.ComponentDiscoveryTest ‑ discover_shouldSkipNonExistentClasses
com.google.firebase.components.ComponentDiscoveryTest ‑ discover_shouldSkipPrivateClasses
com.google.firebase.components.ComponentRuntimeTest ‑ container_shouldExposeAllProvidedInterfacesOfAComponent
com.google.firebase.components.ComponentRuntimeTest ‑ container_shouldExposeAllRegisteredSetValues
com.google.firebase.components.ComponentRuntimeTest ‑ container_withComponentProcessor_shouldDelegateToItForEachComponentRegistrar
…

♻️ This comment has been updated with latest results.

google-oss-bot · 2023-09-06T15:35:46Z

Size Report ¹

Affected Products

firebase-components
Type Base (fe84885) Merge (6b67ec8) Diff
aar 45.1 kB 45.5 kB +368 B (+0.8%)
apk (release) 596 kB 596 kB +4 B (+0.0%)

Test Logs

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

rlazo · 2023-09-06T15:36:16Z

firebase-components/src/main/java/com/google/firebase/components/ComponentRuntime.java

@@ -107,6 +108,8 @@ private void discoverComponents(List<Component<?>> componentsToAdd) {
    // instead of executing such code in the synchronized block below, we store it in a list and
    // execute right after the synchronized section.
    List<Runnable> runAfterDiscovery = new ArrayList<>();
+    List<Component<?>> componentsAdding = new ArrayList<>();


There's one small behavioural change which out of precaution I think we should restore. The current code works as follows

Call discoverComponents with the list of componentsToAdd

During execution, we process pending providers, and modify componentsToAdd

We ensure the list is ok using the CycleDetector

Do what needs to be done with the components

This change no longer updates componentsToAdd to include what is actually going to be processed.

. In the current version, we get as parameter componentsToAdd which is the list of things we have to add

firebase-components/src/main/java/com/google/firebase/components/ComponentRuntime.java

rlazo · 2023-09-06T15:50:11Z

firebase-components/src/main/java/com/google/firebase/components/ComponentRuntime.java

+          }
+        }
+      }
+      for (int i = 0; i < componentsToAdd.size(); i++) {


Do you have any idea how expensive these loops can be? Since this code runs during initialization and, AFAIR in the main thread, any extra work will impact start up for all apps.

An alternative would be to set a class field boolean that works as a flag of whether we have seen the dispatcher before. Then, we would only need to loop once, and we wouldn't need to create an additional list and copy over every component (componentsAdding) nor the list of interfaces already seen (existingInterfaces)

WDYT?

rlazo · 2023-09-06T15:51:38Z

Unit Test Results

  24 files +  18   24 suites +18 16s ⏱️ -2s 115 tests +  97 115 ✔️ +  97 0 💤 ±0 0 ❌ ±0 230 runs +194 230 ✔️ +194 0 💤 ±0 0 ❌ ±0

Results for commit e6481e2. ± Comparison against base commit fe84885.

This pull request removes 18 and adds 115 tests. Note that renamed tests count towards both.
♻️ This comment has been updated with latest results.

@VinayGuthal the "This pull request removes 18 and adds 115 tests. Note that renamed tests count towards both." sounds wrong, any ideas?

firebase-components/src/main/java/com/google/firebase/components/ComponentRuntime.java

rlazo · 2023-09-06T19:05:21Z

Please double check this crash

https://firebase.corp.google.com/project/fireescape-c4819/testlab/histories/bh.150fab898fb191d1/matrices/5367404277897084480/executions/bs.beb53886721b2b11

VinayGuthal requested a review from rlazo September 6, 2023 15:11

google-oss-bot added the size/XL label Sep 6, 2023

VinayGuthal changed the title ~~update component runtime~~ Update component runtime to pick up any one of the couroutine dispatcher interface Sep 6, 2023

update

e6481e2

VinayGuthal force-pushed the component_update branch from c317a90 to e6481e2 Compare September 6, 2023 15:19

google-oss-bot added size/M and removed size/XL labels Sep 6, 2023

rlazo reviewed Sep 6, 2023

View reviewed changes

update components

1be5755

google-oss-bot added size/S and removed size/M labels Sep 6, 2023

VinayGuthal added 2 commits September 6, 2023 14:01

update

204faa6

small nit

4587e61

rlazo reviewed Sep 6, 2023

View reviewed changes

firebase-components/src/main/java/com/google/firebase/components/ComponentRuntime.java Outdated Show resolved Hide resolved

VinayGuthal added 2 commits September 6, 2023 15:07

small fix

9cb493f

update code

748e24c

google-oss-bot added size/M and removed size/S labels Sep 6, 2023

update

8dadad6

google-oss-bot added size/S and removed size/M labels Sep 6, 2023

rlazo approved these changes Sep 6, 2023

View reviewed changes

VinayGuthal added 2 commits September 6, 2023 15:38

nit

a9939a2

add change log entry

8a92f99

VinayGuthal enabled auto-merge (squash) September 6, 2023 19:57

VinayGuthal disabled auto-merge September 6, 2023 20:09

VinayGuthal merged commit b7a34fb into master Sep 6, 2023

VinayGuthal deleted the component_update branch September 6, 2023 20:09

firebase locked and limited conversation to collaborators Oct 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Update component runtime to pick up any one of the couroutine dispatcher interface #5300

Update component runtime to pick up any one of the couroutine dispatcher interface #5300

Uh oh!

VinayGuthal commented Sep 6, 2023

Uh oh!

github-actions bot commented Sep 6, 2023 •

edited

Loading

Uh oh!

google-oss-bot commented Sep 6, 2023 •

edited

Loading

`firebase-components`

Uh oh!

github-actions bot commented Sep 6, 2023 •

edited

Loading

Uh oh!

google-oss-bot commented Sep 6, 2023 •

edited

Loading

`firebase-components`

Uh oh!

rlazo Sep 6, 2023

Uh oh!

Uh oh!

rlazo Sep 6, 2023

Uh oh!

rlazo commented Sep 6, 2023

Unit Test Results

Uh oh!

Uh oh!

rlazo commented Sep 6, 2023

Uh oh!

Uh oh!

Update component runtime to pick up any one of the couroutine dispatcher interface #5300

Update component runtime to pick up any one of the couroutine dispatcher interface #5300

Uh oh!

Conversation

VinayGuthal commented Sep 6, 2023

Uh oh!

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

Uh oh!

Release note changes

Uh oh!

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

Uh oh!

Coverage Report 1

Affected Products

firebase-components

Test Logs

Uh oh!

github-actions bot commented Sep 6, 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 Sep 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Size Report 1

Affected Products

firebase-components

Test Logs

Uh oh!

rlazo Sep 6, 2023

Choose a reason for hiding this comment

Uh oh!

Uh oh!

rlazo Sep 6, 2023

Choose a reason for hiding this comment

Uh oh!

rlazo commented Sep 6, 2023

Unit Test Results

Uh oh!

Uh oh!

rlazo commented Sep 6, 2023

Uh oh!

Uh oh!

github-actions bot commented Sep 6, 2023 •

edited

Loading

google-oss-bot commented Sep 6, 2023 •

edited

Loading

Coverage Report ¹

`firebase-components`

github-actions bot commented Sep 6, 2023 •

edited

Loading

google-oss-bot commented Sep 6, 2023 •

edited

Loading

Size Report ¹

`firebase-components`