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.

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

Implement Android Executors for Storage #4830

Merged
merged 26 commits into from
Apr 26, 2023
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
ff7b3f5
Created four separate pools
maneesht Mar 27, 2023
e01aec4
Used correct executor
maneesht Mar 28, 2023
22a165a
Merge remote-tracking branch 'origin/master' into mtewani/storage-and…
maneesht Mar 28, 2023
694a095
Removed comment and moved pool sizes to static varsZ
maneesht Mar 28, 2023
ad95939
Reverted controllableschedulerhelper
maneesht Mar 28, 2023
e085d58
Reran linter
maneesht Mar 28, 2023
d43b907
Undid firebase-common change
maneesht Mar 29, 2023
7f58222
Addressed comments
maneesht Mar 29, 2023
30c2f2c
WIP
maneesht Mar 30, 2023
89b1303
Merge remote-tracking branch 'origin/master' into mtewani/storage-and…
maneesht Apr 7, 2023
2bf0754
Updated tests to include uithread
maneesht Apr 12, 2023
a2de9b5
WIP
maneesht Apr 12, 2023
336dc6c
Fixed formatting
maneesht Apr 17, 2023
7ed6c14
Merge remote-tracking branch 'origin/master' into mtewani/storage-and…
maneesht Apr 18, 2023
b9a2d4d
Updated retry count
maneesht Apr 18, 2023
b8e467b
Removed unnecessary changes
maneesht Apr 24, 2023
ff4912d
Replaced test statement
maneesht Apr 24, 2023
7e37bf4
Merge branch 'master' into mtewani/storage-android-executor
maneesht Apr 24, 2023
590056a
Added comment and changed throw to a warn
maneesht Apr 24, 2023
13f47a8
Attached buganizer link
maneesht Apr 24, 2023
8a836f1
Debug lines added
maneesht Apr 24, 2023
cf6e553
Fixed formatting
maneesht Apr 25, 2023
0c5062b
Debug prints
maneesht Apr 25, 2023
16535b0
Removed print
maneesht Apr 25, 2023
4ca73c8
Merge branch 'master' into mtewani/storage-android-executor
maneesht Apr 26, 2023
024171d
Removed unnecessary test
maneesht Apr 26, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions firebase-storage/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
# Unreleased
# 20.2.0
* [changed] Replaced custom Thread implementation with Firebase Core executors.

# 20.1.0
* [fixed] Fixed an issue that caused an infinite number of retries with no
Expand Down
2 changes: 1 addition & 1 deletion firebase-storage/firebase-storage.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ android {
}

dependencies {
implementation 'com.google.firebase:firebase-common:20.3.1'
implementation 'com.google.firebase:firebase-common:20.3.2'
implementation 'com.google.firebase:firebase-components:17.1.0'
implementation 'com.google.firebase:firebase-appcheck-interop:16.1.1'

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,13 @@
import androidx.annotation.Nullable;
import androidx.annotation.VisibleForTesting;
import com.google.firebase.FirebaseApp;
import com.google.firebase.annotations.concurrent.Blocking;
import com.google.firebase.appcheck.interop.InternalAppCheckTokenProvider;
import com.google.firebase.auth.internal.InternalAuthProvider;
import com.google.firebase.inject.Provider;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.Executor;

class FirebaseStorageComponent {
/** A map from storage buckets to Firebase Storage instances. */
Expand All @@ -35,10 +37,12 @@ class FirebaseStorageComponent {
FirebaseStorageComponent(
@NonNull FirebaseApp app,
@Nullable Provider<InternalAuthProvider> authProvider,
@Nullable Provider<InternalAppCheckTokenProvider> appCheckProvider) {
@Nullable Provider<InternalAppCheckTokenProvider> appCheckProvider,
@NonNull @Blocking Executor executor) {
this.app = app;
this.authProvider = authProvider;
this.appCheckProvider = appCheckProvider;
StorageTaskScheduler.initializeExecutors(executor);
}

/** Provides instances of Firebase Storage for given bucket names. */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,35 +17,41 @@
import androidx.annotation.Keep;
import androidx.annotation.RestrictTo;
import com.google.firebase.FirebaseApp;
import com.google.firebase.annotations.concurrent.Blocking;
import com.google.firebase.appcheck.interop.InternalAppCheckTokenProvider;
import com.google.firebase.auth.internal.InternalAuthProvider;
import com.google.firebase.components.Component;
import com.google.firebase.components.ComponentRegistrar;
import com.google.firebase.components.Dependency;
import com.google.firebase.components.Qualified;
import com.google.firebase.platforminfo.LibraryVersionComponent;
import java.util.Arrays;
import java.util.List;
import java.util.concurrent.Executor;

/** @hide */
@Keep
@RestrictTo(RestrictTo.Scope.LIBRARY_GROUP)
public class StorageRegistrar implements ComponentRegistrar {
private static final String LIBRARY_NAME = "fire-gcs";
Qualified<Executor> blockingExecutor = Qualified.qualified(Blocking.class, Executor.class);

@Override
public List<Component<?>> getComponents() {
return Arrays.asList(
Component.builder(FirebaseStorageComponent.class)
.name(LIBRARY_NAME)
.add(Dependency.required(FirebaseApp.class))
.add(Dependency.required(blockingExecutor))
.add(Dependency.optionalProvider(InternalAuthProvider.class))
.add(Dependency.optionalProvider(InternalAppCheckTokenProvider.class))
.factory(
c ->
new FirebaseStorageComponent(
c.get(FirebaseApp.class),
c.getProvider(InternalAuthProvider.class),
c.getProvider(InternalAppCheckTokenProvider.class)))
c.getProvider(InternalAppCheckTokenProvider.class),
c.get(blockingExecutor)))
.build(),
LibraryVersionComponent.create(LIBRARY_NAME, BuildConfig.VERSION_NAME));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,10 @@

package com.google.firebase.storage;

import android.annotation.SuppressLint;
import androidx.annotation.NonNull;
import androidx.annotation.RestrictTo;
import java.util.concurrent.BlockingQueue;
import com.google.firebase.concurrent.FirebaseExecutors;
import java.util.concurrent.Executor;
import java.util.concurrent.LinkedBlockingQueue;
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.ThreadPoolExecutor;
import java.util.concurrent.TimeUnit;
import java.util.concurrent.atomic.AtomicInteger;

/**
* A class used to schedule long running operations (upload/download) and operations that are
Expand All @@ -36,48 +30,24 @@
public class StorageTaskScheduler {
public static StorageTaskScheduler sInstance = new StorageTaskScheduler();

private static BlockingQueue<Runnable> mCommandQueue = new LinkedBlockingQueue<>();

// TODO(b/258426744): Migrate to go/firebase-android-executors
@SuppressLint("ThreadPoolCreation")
private static final ThreadPoolExecutor COMMAND_POOL_EXECUTOR =
new ThreadPoolExecutor(
5, 5, 5, TimeUnit.SECONDS, mCommandQueue, new StorageThreadFactory("Command-"));

private static BlockingQueue<Runnable> mUploadQueue = new LinkedBlockingQueue<>();

// TODO(b/258426744): Migrate to go/firebase-android-executors
@SuppressLint("ThreadPoolCreation")
private static final ThreadPoolExecutor UPLOAD_QUEUE_EXECUTOR =
new ThreadPoolExecutor(
2, 2, 5, TimeUnit.SECONDS, mUploadQueue, new StorageThreadFactory("Upload-"));

private static BlockingQueue<Runnable> mDownloadQueue = new LinkedBlockingQueue<>();

// TODO(b/258426744): Migrate to go/firebase-android-executors
@SuppressLint("ThreadPoolCreation")
private static final ThreadPoolExecutor DOWNLOAD_QUEUE_EXECUTOR =
new ThreadPoolExecutor(
3, 3, 5, TimeUnit.SECONDS, mDownloadQueue, new StorageThreadFactory("Download-"));

private static BlockingQueue<Runnable> mCallbackQueue = new LinkedBlockingQueue<>();

// TODO(b/258426744): Migrate to go/firebase-android-executors
@SuppressLint("ThreadPoolCreation")
private static final ThreadPoolExecutor CALLBACK_QUEUE_EXECUTOR =
new ThreadPoolExecutor(
1, 1, 5, TimeUnit.SECONDS, mCallbackQueue, new StorageThreadFactory("Callbacks-"));

static {
COMMAND_POOL_EXECUTOR.allowCoreThreadTimeOut(true);
UPLOAD_QUEUE_EXECUTOR.allowCoreThreadTimeOut(true);
DOWNLOAD_QUEUE_EXECUTOR.allowCoreThreadTimeOut(true);
CALLBACK_QUEUE_EXECUTOR.allowCoreThreadTimeOut(true);
private static final int COMMAND_POOL_SIZE = 5;
private static final int DOWNLOAD_POOL_SIZE = 3;
private static final int UPLOAD_POOL_SIZE = 2;

public static void initializeExecutors(@NonNull Executor firebaseExecutor) {
COMMAND_POOL_EXECUTOR =
FirebaseExecutors.newLimitedConcurrencyExecutor(firebaseExecutor, COMMAND_POOL_SIZE);
DOWNLOAD_QUEUE_EXECUTOR =
FirebaseExecutors.newLimitedConcurrencyExecutor(firebaseExecutor, DOWNLOAD_POOL_SIZE);
UPLOAD_QUEUE_EXECUTOR =
FirebaseExecutors.newLimitedConcurrencyExecutor(firebaseExecutor, UPLOAD_POOL_SIZE);
CALLBACK_QUEUE_EXECUTOR = FirebaseExecutors.newSequentialExecutor(firebaseExecutor);
}

public static void setCallbackQueueKeepAlive(long keepAliveTime, TimeUnit timeUnit) {
CALLBACK_QUEUE_EXECUTOR.setKeepAliveTime(keepAliveTime, timeUnit);
}
private static Executor COMMAND_POOL_EXECUTOR;
private static Executor UPLOAD_QUEUE_EXECUTOR;
private static Executor DOWNLOAD_QUEUE_EXECUTOR;
private static Executor CALLBACK_QUEUE_EXECUTOR;

public static StorageTaskScheduler getInstance() {
return sInstance;
Expand All @@ -102,27 +72,4 @@ public void scheduleCallback(Runnable task) {
public Executor getCommandPoolExecutor() {
return COMMAND_POOL_EXECUTOR;
}

/** The thread factory for Storage threads. */
static class StorageThreadFactory implements ThreadFactory {
private final AtomicInteger threadNumber = new AtomicInteger(1);
private final String mNameSuffix;

StorageThreadFactory(@NonNull String suffix) {
mNameSuffix = suffix;
}

@Override
@SuppressWarnings("ThreadPriorityCheck")
// TODO(b/258426744): Migrate to go/firebase-android-executors
@SuppressLint("ThreadPoolCreation")
public Thread newThread(@NonNull Runnable r) {
Thread t = new Thread(r, "FirebaseStorage-" + mNameSuffix + threadNumber.getAndIncrement());
t.setDaemon(false);
t.setPriority(
android.os.Process.THREAD_PRIORITY_BACKGROUND
+ android.os.Process.THREAD_PRIORITY_MORE_FAVORABLE);
return t;
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,6 @@
public class TestUtil {

static FirebaseApp createApp() {
/**
* Many tests require you to call the callback on the same thread that was initially
* instantiated. With the 5 second keepalive, after 5 seconds, the thread will get killed and
* eventually a new one will be created. Therefore causing many of the tests to fail.
*/
StorageTaskScheduler.setCallbackQueueKeepAlive(90, TimeUnit.SECONDS);
return FirebaseApp.initializeApp(
ApplicationProvider.getApplicationContext(),
new FirebaseOptions.Builder()
Expand Down