Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Android 34 WorkManager Scheduler #6221

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Android 34 WorkManager Scheduler #6221

wants to merge 10 commits into from

Conversation

emilypgoogle
Copy link
Collaborator

No description provided.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 28, 2024 •
edited
Loading

Release note changes

No release note changes were detected. If you made changes that should be
present in the next release, ensure you've added an entry in the appropriate
CHANGELOG.md file(s).

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Aug 28, 2024 •
edited
Loading

Coverage Report 1

Affected Products

  • transport-runtime

    Overall coverage changed from ? (adab25c) to 57.95% (611ef21) by ?.

    95 individual files with coverage change

    FilenameBase (adab25c)Merge (611ef21)Diff
    AlarmManagerScheduler.java?90.91%?
    AlarmManagerSchedulerBroadcastReceiver.java?6.25%?
    AutoProtoEncoderDoNotUseEncoder.java?84.47%?
    AutoValue_BackendRequest.java?60.00%?
    AutoValue_BackendResponse.java?31.82%?
    AutoValue_CreationContext.java?55.26%?
    AutoValue_EventInternal.java?66.06%?
    AutoValue_EventStoreConfig.java?56.06%?
    AutoValue_PersistedEvent.java?55.17%?
    AutoValue_SchedulerConfig.java?33.33%?
    AutoValue_SchedulerConfig_ConfigValue.java?53.19%?
    AutoValue_SendRequest.java?63.38%?
    AutoValue_TransportContext.java?86.36%?
    BackendFactory.java?0.00%?
    BackendRegistry.java?0.00%?
    BackendRegistryModule.java?0.00%?
    BackendRequest.java?100.00%?
    BackendResponse.java?100.00%?
    ClientHealthMetricsStore.java?0.00%?
    ClientMetrics.java?57.14%?
    Clock.java?0.00%?
    CreationContext.java?66.67%?
    CreationContextFactory.java?100.00%?
    CreationContextFactory_Factory.java?0.00%?
    DaggerTransportRuntimeComponent.java?0.00%?
    DefaultScheduler.java?80.00%?
    DefaultScheduler_Factory.java?0.00%?
    Destination.java?0.00%?
    EncodedDestination.java?0.00%?
    EncodedPayload.java?61.90%?
    EventInternal.java?60.00%?
    EventStore.java?0.00%?
    EventStoreConfig.java?100.00%?
    EventStoreModule.java?0.00%?
    EventStoreModule_DbNameFactory.java?0.00%?
    EventStoreModule_PackageNameFactory.java?0.00%?
    EventStoreModule_SchemaVersionFactory.java?0.00%?
    EventStoreModule_StoreConfigFactory.java?0.00%?
    ExecutionModule.java?0.00%?
    ExecutionModule_ExecutorFactory.java?0.00%?
    ForcedSender.java?0.00%?
    Function.java?0.00%?
    GlobalMetrics.java?0.00%?
    JobInfoScheduler.java?95.00%?
    JobInfoSchedulerService.java?0.00%?
    LogEventDropped.java?40.00%?
    Logging.java?47.06%?
    LogSourceMetrics.java?0.00%?
    MetadataBackendRegistry.java?66.67%?
    MetadataBackendRegistry_Factory.java?0.00%?
    Monotonic.java?0.00%?
    PersistedEvent.java?100.00%?
    PriorityMapping.java?88.89%?
    ProtoEncoderDoNotUse.java?50.00%?
    Retries.java?0.00%?
    RetryStrategy.java?0.00%?
    SafeLoggingExecutor.java?84.62%?
    Scheduler.java?0.00%?
    SchedulerConfig.java?98.53%?
    SchedulingConfigModule.java?0.00%?
    SchedulingConfigModule_ConfigFactory.java?0.00%?
    SchedulingModule.java?0.00%?
    SchedulingModule_WorkSchedulerFactory.java?0.00%?
    SchemaManager.java?98.65%?
    SchemaManager_Factory.java?0.00%?
    SendRequest.java?100.00%?
    SQLiteEventStore.java?39.16%?
    SQLiteEventStore_Factory.java?0.00%?
    StorageMetrics.java?0.00%?
    SynchronizationException.java?0.00%?
    SynchronizationGuard.java?0.00%?
    TestClock.java?40.00%?
    TimeModule.java?0.00%?
    TimeModule_EventClockFactory.java?0.00%?
    TimeModule_UptimeClockFactory.java?0.00%?
    TimeWindow.java?0.00%?
    TransportBackend.java?0.00%?
    TransportBackendDiscovery.java?0.00%?
    TransportContext.java?69.23%?
    TransportFactoryImpl.java?100.00%?
    TransportImpl.java?84.21%?
    TransportInternal.java?0.00%?
    TransportRuntime.java?66.15%?
    TransportRuntimeComponent.java?0.00%?
    TransportRuntime_Factory.java?0.00%?
    Uploader.java?95.45%?
    Uploader_Factory.java?0.00%?
    UptimeClock.java?100.00%?
    WallTime.java?0.00%?
    WallTimeClock.java?0.00%?
    WorkInitializer.java?0.00%?
    WorkInitializer_Factory.java?0.00%?
    WorkManagerScheduler.java?0.00%?
    WorkManagerSchedulerWorker.java?0.00%?
    WorkScheduler.java?100.00%?

Test Logs

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

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Aug 28, 2024 •
edited
Loading

Unit Test Results

  12 files   -      956    12 suites   - 956   16s ⏱️ - 35m 10s
  50 tests  -   5 273    50 ✔️  -   5 252  0 💤  - 21  0 ❌ ±0 
100 runs   - 10 631  100 ✔️  - 10 589  0 💤  - 42  0 ❌ ±0 

Results for commit 0564a10. ± Comparison against base commit 456f431.

♻️ This comment has been updated with latest results.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Aug 28, 2024 •
edited
Loading

Size Report 1

Affected Products

  • transport-runtime

    TypeBase (adab25c)Merge (611ef21)Diff
    aar163 kB167 kB+4.58 kB (+2.8%)
    apk (aggressive)55.7 kB492 kB+436 kB (+783.2%)
    apk (release)193 kB4.99 MB+4.80 MB (+2484.4%)

Test Logs

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

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Aug 28, 2024 •
edited
Loading

Startup Time Report 1

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

Startup time comparison between the CI merge commit (611ef21) and the base commit (adab25c) are not available.

No macrobenchmark data found for the base commit (adab25c). Analysis for the CI merge commit (611ef21) can be found at:

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

rlazo
rlazo requested changes Aug 30, 2024
View reviewed changes
.../com/google/android/datatransport/runtime/scheduling/jobscheduling/WorkManagerScheduler.java Outdated Show resolved Hide resolved
.../com/google/android/datatransport/runtime/scheduling/jobscheduling/WorkManagerScheduler.java Outdated Show resolved Hide resolved
.../com/google/android/datatransport/runtime/scheduling/jobscheduling/WorkManagerScheduler.java Show resolved Hide resolved
.../com/google/android/datatransport/runtime/scheduling/jobscheduling/WorkManagerScheduler.java Outdated Show resolved Hide resolved
.../com/google/android/datatransport/runtime/scheduling/jobscheduling/WorkManagerScheduler.java Outdated
int jobId = WorkScheduler.getJobId(context, transportContext);
if (!force) {
try {
for (WorkInfo info : manager.getWorkInfosByTag("transport-" + jobId).get()) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

getWorkInfosByTag returns a list, but it should always be exactly 1, right? If that's the case, we need to verify that condition, as if it's not true something else has gone wrong, instead of looping through it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not sure, maybe it keeps finished jobs for some period of time? It wouldn't be a condition we verify though, scheduling can be called with force = true beforehand and schedule multiple before calling with force = false, I think.

emilypgoogle and others added 3 commits August 30, 2024 16:08
@emilypgoogle @rlazo
Update transport/transport-runtime/src/main/java/com/google/android/d…
15c5c90 
…atatransport/runtime/scheduling/jobscheduling/WorkManagerScheduler.java

Co-authored-by: Rodrigo Lazo <rlazo@users.noreply.github.com>
@emilypgoogle
Tag and exception
bd1ad49
@emilypgoogle
Organize imports
0564a10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidmotson davidmotson davidmotson approved these changes

@rlazo rlazo Awaiting requested review from rlazo

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
size/L
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@emilypgoogle @google-oss-bot @rlazo @davidmotson