Skip to content

Register executors as components. #4288

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 7 commits into from
Nov 10, 2022
Merged

Register executors as components. #4288

merged 7 commits into from
Nov 10, 2022

Conversation

vkryachko
Copy link
Member

The intent for those is to be used by all Firebase SDKs and forbid creating their own at will.

@google-oss-bot
Copy link
Contributor

1 Warning
⚠️ Did you forget to add a changelog entry? (Add the 'no-changelog' label to the PR to silence this warning.)

Generated by 🚫 Danger

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Nov 7, 2022

Coverage Report 1

Affected Products

  • firebase-common

    Overall coverage changed from ? (fe5b2f3) to 65.46% (36a58a0) by ?.

    37 individual files with coverage change

    FilenameBase (fe5b2f3)Merge (36a58a0)Diff
    AutoValue_HeartBeatResult.java?33.33%?
    AutoValue_LibraryVersion.java?58.33%?
    AutoValue_SdkHeartBeatResult.java?0.00%?
    ComponentDiscoveryService.java?0.00%?
    ComponentMonitor.java?100.00%?
    CustomThreadFactory.java?54.55%?
    DataCollectionConfigStorage.java?88.89%?
    DataCollectionDefaultChange.java?100.00%?
    DefaultHeartBeatController.java?95.59%?
    DefaultUserAgentPublisher.java?95.45%?
    DelegatingScheduledExecutorService.java?22.64%?
    DelegatingScheduledFuture.java?69.23%?
    EmulatedServiceSettings.java?0.00%?
    ExecutorsRegistrar.java?100.00%?
    FirebaseApp.java?56.02%?
    FirebaseAppLifecycleListener.java?0.00%?
    FirebaseCommonRegistrar.java?95.74%?
    FirebaseError.java?0.00%?
    FirebaseInitProvider.java?58.82%?
    FirebaseNetworkException.java?0.00%?
    FirebaseOptions.java?31.94%?
    FirebaseTooManyRequestsException.java?0.00%?
    FirebaseTrace.java?100.00%?
    GlobalLibraryVersionRegistrar.java?75.00%?
    HeartBeatConsumer.java?0.00%?
    HeartBeatConsumerComponent.java?0.00%?
    HeartBeatController.java?0.00%?
    HeartBeatInfo.java?100.00%?
    HeartBeatInfoStorage.java?90.57%?
    HeartBeatResult.java?100.00%?
    KotlinDetector.java?66.67%?
    LibraryVersion.java?100.00%?
    LibraryVersionComponent.java?100.00%?
    PublicApi.java?0.00%?
    SdkHeartBeatResult.java?0.00%?
    UiExecutor.java?60.00%?
    UserAgentPublisher.java?0.00%?

Test Logs

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

@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2022

Unit Test Results

   384 files     384 suites   18m 15s ⏱️
4 683 tests 4 660 ✔️ 22 💤 1
4 699 runs  4 676 ✔️ 22 💤 1

For more details on these failures, see this check.

Results for commit ae742a0.

♻️ This comment has been updated with latest results.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Nov 7, 2022

Size Report 1

Affected Products

  • firebase-annotations

    TypeBase (fe5b2f3)Merge (e216fe0)Diff
    apk (release)8.89 kB9.46 kB+568 B (+6.4%)
  • firebase-common

    TypeBase (fe5b2f3)Merge (e216fe0)Diff
    aar50.1 kB58.3 kB+8.14 kB (+16.2%)
    apk (aggressive)85.6 kB94.3 kB+8.65 kB (+10.1%)
    apk (release)684 kB696 kB+12.1 kB (+1.8%)
  • firebase-common-ktx

    TypeBase (fe5b2f3)Merge (e216fe0)Diff
    aar6.03 kB13.5 kB+7.48 kB (+124.1%)
    apk (aggressive)110 kB122 kB+12.4 kB (+11.3%)
    apk (release)1.62 MB1.63 MB+14.2 kB (+0.9%)
  • firebase-components

    TypeBase (fe5b2f3)Merge (e216fe0)Diff
    apk (release)33.2 kB33.6 kB+368 B (+1.1%)

Test Logs

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

Base automatically changed from vk.component_qualifiers to executors November 8, 2022 16:23
The intent for those is to be used by all Firebase SDKs and forbid
creating their own at will.

class CustomThreadFactory implements ThreadFactory {
private static final ThreadFactory DEFAULT = Executors.defaultThreadFactory();
private final AtomicLong threadCount = new AtomicLong();
Copy link
Collaborator

Choose a reason for hiding this comment

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

why the counter?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is common practice to give different names to threads, i.e. executor-foo-1, executor-foo-2 etc.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Didn't see it in the name, thanks!

@vkryachko vkryachko merged commit 3cf020a into executors Nov 10, 2022
@vkryachko vkryachko deleted the vk.executors branch November 10, 2022 17:36
vkryachko added a commit that referenced this pull request Nov 15, 2022
* Add qualifier support to firebase components. (#3180)

* Add qualifier support to firebase components.

Details: go/firebase-component-qualifiers

* fix errorprone error.

* change copyright year.

* Register executors as components. (#4288)

* Register executors as components.

The intent for those is to be used by all Firebase SDKs and forbid
creating their own at will.

* Add copyrights.

* add more copyrights

* ktlintformat

* gJF

* ktlint

* Address review comments.

* Adds generally useful executors (#4305)

Namely, SequentialExecutor and directExecutor.

* Enable strict mode for executors. (#4303)

Any violations would kill the app in debug builds of firebase-common,
and log a warning in release builds. This is done to fail tests that
incorrectly use executors while not affecting 3p apps in release builds.

Additionally correctly set thread priorities in an Android specific way.

* Enable thread pool linter check. (#4297)

* Enable thread pool linter check.

All violations are now suppressed, bugs filed to fix each product.

* ktlint

* Remove init

* Fix copyright
davidmotson pushed a commit that referenced this pull request Nov 28, 2022
* Add qualifier support to firebase components. (#3180)

* Add qualifier support to firebase components.

Details: go/firebase-component-qualifiers

* fix errorprone error.

* change copyright year.

* Register executors as components. (#4288)

* Register executors as components.

The intent for those is to be used by all Firebase SDKs and forbid
creating their own at will.

* Add copyrights.

* add more copyrights

* ktlintformat

* gJF

* ktlint

* Address review comments.

* Adds generally useful executors (#4305)

Namely, SequentialExecutor and directExecutor.

* Enable strict mode for executors. (#4303)

Any violations would kill the app in debug builds of firebase-common,
and log a warning in release builds. This is done to fail tests that
incorrectly use executors while not affecting 3p apps in release builds.

Additionally correctly set thread priorities in an Android specific way.

* Enable thread pool linter check. (#4297)

* Enable thread pool linter check.

All violations are now suppressed, bugs filed to fix each product.

* ktlint

* Remove init

* Fix copyright
@firebase firebase locked and limited conversation to collaborators Dec 11, 2022
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