Update component runtime to pick up any one of the couroutine dispatcher interface #5300
Conversation
Release note changesThe following had changelogs that were modified, but did not have any unreleased entries for release notes to generate from. Changelogsfirebase-components |
VinayGuthal changed the title 
Coverage Report 1Affected Products
Test Logs |
Unit Test Results 24 files + 18 24 suites +18 16s ⏱️ -2s 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.♻️ This comment has been updated with latest results. |
VinayGuthal force-pushed the component_update branch from c317a90 to e6481e2 Compare Size Report 1Affected Products
Test Logs |
| // 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 was a problem hiding this comment.
There's one small behavioural change which out of precaution I think we should restore. The current code works as follows
- Call
discoverComponentswith the list ofcomponentsToAdd - 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 Show resolved Hide resolved
| } | ||
| } | ||
| } | ||
| for (int i = 0; i < componentsToAdd.size(); i++) { |
There was a problem hiding this comment.
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?
@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 Outdated Show resolved Hide resolved
3 participants
No description provided.