diff --git a/apex/appsearch/service/java/com/android/server/appsearch/AppSearchManagerService.java b/apex/appsearch/service/java/com/android/server/appsearch/AppSearchManagerService.java index c7c1d68b90f19..e5244296b1f47 100644 --- a/apex/appsearch/service/java/com/android/server/appsearch/AppSearchManagerService.java +++ b/apex/appsearch/service/java/com/android/server/appsearch/AppSearchManagerService.java @@ -348,19 +348,19 @@ public class AppSearchManagerService extends SystemService { 2 * (int) (totalLatencyStartTimeMillis - binderCallStartTimeMillis); int totalLatencyMillis = (int) (SystemClock.elapsedRealtime() - totalLatencyStartTimeMillis); - CallStats.Builder cBuilder = new CallStats.Builder(packageName, - databaseName) + logger.logStats(new CallStats.Builder() + .setPackageName(packageName) + .setDatabase(databaseName) + .setStatusCode(statusCode) + .setTotalLatencyMillis(totalLatencyMillis) .setCallType(CallStats.CALL_TYPE_SET_SCHEMA) // TODO(b/173532925) check the existing binder call latency chart // is good enough for us: // http://dashboards/view/_72c98f9a_91d9_41d4_ab9a_bc14f79742b4 .setEstimatedBinderLatencyMillis(estimatedBinderLatencyMillis) .setNumOperationsSucceeded(operationSuccessCount) - .setNumOperationsFailed(operationFailureCount); - cBuilder.getGeneralStatsBuilder() - .setStatusCode(statusCode) - .setTotalLatencyMillis(totalLatencyMillis); - logger.logStats(cBuilder.build()); + .setNumOperationsFailed(operationFailureCount) + .build()); } } }); @@ -480,19 +480,19 @@ public class AppSearchManagerService extends SystemService { 2 * (int) (totalLatencyStartTimeMillis - binderCallStartTimeMillis); int totalLatencyMillis = (int) (SystemClock.elapsedRealtime() - totalLatencyStartTimeMillis); - CallStats.Builder cBuilder = new CallStats.Builder(packageName, - databaseName) + logger.logStats(new CallStats.Builder() + .setPackageName(packageName) + .setDatabase(databaseName) + .setStatusCode(statusCode) + .setTotalLatencyMillis(totalLatencyMillis) .setCallType(CallStats.CALL_TYPE_PUT_DOCUMENTS) // TODO(b/173532925) check the existing binder call latency chart // is good enough for us: // http://dashboards/view/_72c98f9a_91d9_41d4_ab9a_bc14f79742b4 .setEstimatedBinderLatencyMillis(estimatedBinderLatencyMillis) .setNumOperationsSucceeded(operationSuccessCount) - .setNumOperationsFailed(operationFailureCount); - cBuilder.getGeneralStatsBuilder() - .setStatusCode(statusCode) - .setTotalLatencyMillis(totalLatencyMillis); - logger.logStats(cBuilder.build()); + .setNumOperationsFailed(operationFailureCount) + .build()); } } }); @@ -563,19 +563,19 @@ public class AppSearchManagerService extends SystemService { 2 * (int) (totalLatencyStartTimeMillis - binderCallStartTimeMillis); int totalLatencyMillis = (int) (SystemClock.elapsedRealtime() - totalLatencyStartTimeMillis); - CallStats.Builder cBuilder = new CallStats.Builder(packageName, - databaseName) + logger.logStats(new CallStats.Builder() + .setPackageName(packageName) + .setDatabase(databaseName) + .setStatusCode(statusCode) + .setTotalLatencyMillis(totalLatencyMillis) .setCallType(CallStats.CALL_TYPE_GET_DOCUMENTS) // TODO(b/173532925) check the existing binder call latency chart // is good enough for us: // http://dashboards/view/_72c98f9a_91d9_41d4_ab9a_bc14f79742b4 .setEstimatedBinderLatencyMillis(estimatedBinderLatencyMillis) .setNumOperationsSucceeded(operationSuccessCount) - .setNumOperationsFailed(operationFailureCount); - cBuilder.getGeneralStatsBuilder() - .setStatusCode(statusCode) - .setTotalLatencyMillis(totalLatencyMillis); - logger.logStats(cBuilder.build()); + .setNumOperationsFailed(operationFailureCount) + .build()); } } }); @@ -631,19 +631,19 @@ public class AppSearchManagerService extends SystemService { 2 * (int) (totalLatencyStartTimeMillis - binderCallStartTimeMillis); int totalLatencyMillis = (int) (SystemClock.elapsedRealtime() - totalLatencyStartTimeMillis); - CallStats.Builder cBuilder = new CallStats.Builder(packageName, - databaseName) + logger.logStats(new CallStats.Builder() + .setPackageName(packageName) + .setDatabase(databaseName) + .setStatusCode(statusCode) + .setTotalLatencyMillis(totalLatencyMillis) .setCallType(CallStats.CALL_TYPE_SEARCH) // TODO(b/173532925) check the existing binder call latency chart // is good enough for us: // http://dashboards/view/_72c98f9a_91d9_41d4_ab9a_bc14f79742b4 .setEstimatedBinderLatencyMillis(estimatedBinderLatencyMillis) .setNumOperationsSucceeded(operationSuccessCount) - .setNumOperationsFailed(operationFailureCount); - cBuilder.getGeneralStatsBuilder() - .setStatusCode(statusCode) - .setTotalLatencyMillis(totalLatencyMillis); - logger.logStats(cBuilder.build()); + .setNumOperationsFailed(operationFailureCount) + .build()); } } }); @@ -697,20 +697,18 @@ public class AppSearchManagerService extends SystemService { 2 * (int) (totalLatencyStartTimeMillis - binderCallStartTimeMillis); int totalLatencyMillis = (int) (SystemClock.elapsedRealtime() - totalLatencyStartTimeMillis); - // TODO(b/173532925) database would be nulluable once we remove generalStats - CallStats.Builder cBuilder = new CallStats.Builder(packageName, - /*database=*/ "") + logger.logStats(new CallStats.Builder() + .setPackageName(packageName) + .setStatusCode(statusCode) + .setTotalLatencyMillis(totalLatencyMillis) .setCallType(CallStats.CALL_TYPE_GLOBAL_SEARCH) // TODO(b/173532925) check the existing binder call latency chart // is good enough for us: // http://dashboards/view/_72c98f9a_91d9_41d4_ab9a_bc14f79742b4 .setEstimatedBinderLatencyMillis(estimatedBinderLatencyMillis) .setNumOperationsSucceeded(operationSuccessCount) - .setNumOperationsFailed(operationFailureCount); - cBuilder.getGeneralStatsBuilder() - .setStatusCode(statusCode) - .setTotalLatencyMillis(totalLatencyMillis); - logger.logStats(cBuilder.build()); + .setNumOperationsFailed(operationFailureCount) + .build()); } } }); @@ -965,19 +963,19 @@ public class AppSearchManagerService extends SystemService { 2 * (int) (totalLatencyStartTimeMillis - binderCallStartTimeMillis); int totalLatencyMillis = (int) (SystemClock.elapsedRealtime() - totalLatencyStartTimeMillis); - CallStats.Builder cBuilder = new CallStats.Builder(packageName, - databaseName) + logger.logStats(new CallStats.Builder() + .setPackageName(packageName) + .setDatabase(databaseName) + .setStatusCode(statusCode) + .setTotalLatencyMillis(totalLatencyMillis) .setCallType(CallStats.CALL_TYPE_REMOVE_DOCUMENTS_BY_ID) // TODO(b/173532925) check the existing binder call latency chart // is good enough for us: // http://dashboards/view/_72c98f9a_91d9_41d4_ab9a_bc14f79742b4 .setEstimatedBinderLatencyMillis(estimatedBinderLatencyMillis) .setNumOperationsSucceeded(operationSuccessCount) - .setNumOperationsFailed(operationFailureCount); - cBuilder.getGeneralStatsBuilder() - .setStatusCode(statusCode) - .setTotalLatencyMillis(totalLatencyMillis); - logger.logStats(cBuilder.build()); + .setNumOperationsFailed(operationFailureCount) + .build()); } } }); @@ -1033,19 +1031,19 @@ public class AppSearchManagerService extends SystemService { 2 * (int) (totalLatencyStartTimeMillis - binderCallStartTimeMillis); int totalLatencyMillis = (int) (SystemClock.elapsedRealtime() - totalLatencyStartTimeMillis); - CallStats.Builder cBuilder = new CallStats.Builder(packageName, - databaseName) + logger.logStats(new CallStats.Builder() + .setPackageName(packageName) + .setDatabase(databaseName) + .setStatusCode(statusCode) + .setTotalLatencyMillis(totalLatencyMillis) .setCallType(CallStats.CALL_TYPE_REMOVE_DOCUMENTS_BY_SEARCH) // TODO(b/173532925) check the existing binder call latency chart // is good enough for us: // http://dashboards/view/_72c98f9a_91d9_41d4_ab9a_bc14f79742b4 .setEstimatedBinderLatencyMillis(estimatedBinderLatencyMillis) .setNumOperationsSucceeded(operationSuccessCount) - .setNumOperationsFailed(operationFailureCount); - cBuilder.getGeneralStatsBuilder() - .setStatusCode(statusCode) - .setTotalLatencyMillis(totalLatencyMillis); - logger.logStats(cBuilder.build()); + .setNumOperationsFailed(operationFailureCount) + .build()); } } }); @@ -1110,19 +1108,17 @@ public class AppSearchManagerService extends SystemService { 2 * (int) (totalLatencyStartTimeMillis - binderCallStartTimeMillis); int totalLatencyMillis = (int) (SystemClock.elapsedRealtime() - totalLatencyStartTimeMillis); - CallStats.Builder cBuilder = new CallStats.Builder(/*packageName=*/ "", - /*databaseName=*/ "") + logger.logStats(new CallStats.Builder() + .setStatusCode(statusCode) + .setTotalLatencyMillis(totalLatencyMillis) .setCallType(CallStats.CALL_TYPE_FLUSH) // TODO(b/173532925) check the existing binder call latency chart // is good enough for us: // http://dashboards/view/_72c98f9a_91d9_41d4_ab9a_bc14f79742b4 .setEstimatedBinderLatencyMillis(estimatedBinderLatencyMillis) .setNumOperationsSucceeded(operationSuccessCount) - .setNumOperationsFailed(operationFailureCount); - cBuilder.getGeneralStatsBuilder() - .setStatusCode(statusCode) - .setTotalLatencyMillis(totalLatencyMillis); - logger.logStats(cBuilder.build()); + .setNumOperationsFailed(operationFailureCount) + .build()); } } }); @@ -1162,21 +1158,17 @@ public class AppSearchManagerService extends SystemService { 2 * (int) (totalLatencyStartTimeMillis - binderCallStartTimeMillis); int totalLatencyMillis = (int) (SystemClock.elapsedRealtime() - totalLatencyStartTimeMillis); - // TODO(b/173532925) make packageName and database nullable after - // removing generalStats - CallStats.Builder cBuilder = new CallStats.Builder(/*packageName=*/"", - /*database=*/ "") + logger.logStats(new CallStats.Builder() + .setStatusCode(statusCode) + .setTotalLatencyMillis(totalLatencyMillis) .setCallType(CallStats.CALL_TYPE_INITIALIZE) // TODO(b/173532925) check the existing binder call latency chart // is good enough for us: // http://dashboards/view/_72c98f9a_91d9_41d4_ab9a_bc14f79742b4 .setEstimatedBinderLatencyMillis(estimatedBinderLatencyMillis) .setNumOperationsSucceeded(operationSuccessCount) - .setNumOperationsFailed(operationFailureCount); - cBuilder.getGeneralStatsBuilder() - .setStatusCode(statusCode) - .setTotalLatencyMillis(totalLatencyMillis); - logger.logStats(cBuilder.build()); + .setNumOperationsFailed(operationFailureCount) + .build()); } } }); diff --git a/apex/appsearch/service/java/com/android/server/appsearch/ImplInstanceManager.java b/apex/appsearch/service/java/com/android/server/appsearch/ImplInstanceManager.java index e7845d51568d5..289413aebbb51 100644 --- a/apex/appsearch/service/java/com/android/server/appsearch/ImplInstanceManager.java +++ b/apex/appsearch/service/java/com/android/server/appsearch/ImplInstanceManager.java @@ -28,6 +28,7 @@ import android.util.ArrayMap; import com.android.internal.annotations.GuardedBy; import com.android.server.appsearch.external.localstorage.AppSearchImpl; import com.android.server.appsearch.external.localstorage.AppSearchLogger; +import com.android.server.appsearch.external.localstorage.FrameworkOptimizeStrategy; import java.io.File; import java.util.Map; @@ -153,6 +154,10 @@ public final class ImplInstanceManager { @Nullable AppSearchLogger logger) throws AppSearchException { File appSearchDir = getAppSearchDir(userHandle); - return AppSearchImpl.create(appSearchDir, userContext, /*logger=*/ null); + return AppSearchImpl.create( + appSearchDir, + userContext, + /*logger=*/ null, + new FrameworkOptimizeStrategy()); } } diff --git a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/AppSearchImpl.java b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/AppSearchImpl.java index 29cb57c05eebc..4a1a9ae3546be 100644 --- a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/AppSearchImpl.java +++ b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/AppSearchImpl.java @@ -145,14 +145,14 @@ import java.util.concurrent.locks.ReentrantReadWriteLock; public final class AppSearchImpl implements Closeable { private static final String TAG = "AppSearchImpl"; - @VisibleForTesting static final int OPTIMIZE_THRESHOLD_DOC_COUNT = 1000; - @VisibleForTesting static final int OPTIMIZE_THRESHOLD_BYTES = 1_000_000; // 1MB @VisibleForTesting static final int CHECK_OPTIMIZE_INTERVAL = 100; private final ReadWriteLock mReadWriteLock = new ReentrantReadWriteLock(); private final LogUtil mLogUtil = new LogUtil(TAG); + private final OptimizeStrategy mOptimizeStrategy; + @GuardedBy("mReadWriteLock") @VisibleForTesting final IcingSearchEngine mIcingSearchEngineLocked; @@ -201,10 +201,12 @@ public final class AppSearchImpl implements Closeable { public static AppSearchImpl create( @NonNull File icingDir, @NonNull Context userContext, - @Nullable AppSearchLogger logger) + @Nullable AppSearchLogger logger, + @NonNull OptimizeStrategy optimizeStrategy) throws AppSearchException { Objects.requireNonNull(icingDir); Objects.requireNonNull(userContext); + Objects.requireNonNull(optimizeStrategy); long totalLatencyStartMillis = SystemClock.elapsedRealtime(); InitializeStats.Builder initStatsBuilder = null; @@ -212,7 +214,9 @@ public final class AppSearchImpl implements Closeable { initStatsBuilder = new InitializeStats.Builder(); } - AppSearchImpl appSearchImpl = new AppSearchImpl(icingDir, userContext, initStatsBuilder); + AppSearchImpl appSearchImpl = + new AppSearchImpl( + icingDir, userContext, initStatsBuilder, optimizeStrategy); long prepareVisibilityStoreLatencyStartMillis = SystemClock.elapsedRealtime(); appSearchImpl.initializeVisibilityStore(); @@ -236,7 +240,8 @@ public final class AppSearchImpl implements Closeable { private AppSearchImpl( @NonNull File icingDir, @NonNull Context userContext, - @Nullable InitializeStats.Builder initStatsBuilder) + @Nullable InitializeStats.Builder initStatsBuilder, + @NonNull OptimizeStrategy optimizeStrategy) throws AppSearchException { mReadWriteLock.writeLock().lock(); @@ -254,6 +259,7 @@ public final class AppSearchImpl implements Closeable { Objects.hashCode(mIcingSearchEngineLocked)); mVisibilityStoreLocked = new VisibilityStore(this, userContext); + mOptimizeStrategy = optimizeStrategy; // The core initialization procedure. If any part of this fails, we bail into // resetLocked(), deleting all data (but hopefully allowing AppSearchImpl to come up). @@ -646,9 +652,7 @@ public final class AppSearchImpl implements Closeable { // Logging stats if (pStatsBuilder != null) { pStatsBuilder - .getGeneralStatsBuilder() - .setStatusCode(statusProtoToResultCode(putResultProto.getStatus())); - pStatsBuilder + .setStatusCode(statusProtoToResultCode(putResultProto.getStatus())) .setGenerateDocumentProtoLatencyMillis( (int) (generateDocumentProtoEndTimeMillis @@ -667,9 +671,8 @@ public final class AppSearchImpl implements Closeable { if (logger != null) { long totalEndTimeMillis = SystemClock.elapsedRealtime(); - pStatsBuilder - .getGeneralStatsBuilder() - .setTotalLatencyMillis((int) (totalEndTimeMillis - totalStartTimeMillis)); + pStatsBuilder.setTotalLatencyMillis( + (int) (totalEndTimeMillis - totalStartTimeMillis)); logger.logStats(pStatsBuilder.build()); } } @@ -812,7 +815,8 @@ public final class AppSearchImpl implements Closeable { * * @param queryExpression Query String to search. * @param searchSpec Spec for setting filters, raw query etc. - * @param callerPackageName Package name of the caller, should belong to the {@code callerUid}. + * @param callerPackageName Package name of the caller, should belong to the {@code + * userContext}. * @param callerUid UID of the client making the globalQuery call. * @param logger logger to collect globalQuery stats * @return The results of performing this search. It may contain an empty list of results if no @@ -2001,7 +2005,7 @@ public final class AppSearchImpl implements Closeable { * resources that could be released. * *

{@link IcingSearchEngine#optimize()} should be called only if {@link - * GetOptimizeInfoResultProto} shows there is enough resources could be released. + * OptimizeStrategy#shouldOptimize(GetOptimizeInfoResultProto)} return true. */ public void checkForOptimize() throws AppSearchException { mReadWriteLock.writeLock().lock(); @@ -2009,9 +2013,7 @@ public final class AppSearchImpl implements Closeable { GetOptimizeInfoResultProto optimizeInfo = getOptimizeInfoResultLocked(); checkSuccess(optimizeInfo.getStatus()); mOptimizeIntervalCountLocked = 0; - // Second threshold, decide when to call optimize(). - if (optimizeInfo.getOptimizableDocs() >= OPTIMIZE_THRESHOLD_DOC_COUNT - || optimizeInfo.getEstimatedOptimizableBytes() >= OPTIMIZE_THRESHOLD_BYTES) { + if (mOptimizeStrategy.shouldOptimize(optimizeInfo)) { optimize(); } } finally { @@ -2022,11 +2024,7 @@ public final class AppSearchImpl implements Closeable { // go/icing-library-apis. } - /** - * Triggers {@link IcingSearchEngine#optimize()} directly. - * - *

This method should be only called as a scheduled task in AppSearch Platform backend. - */ + /** Triggers {@link IcingSearchEngine#optimize()} directly. */ public void optimize() throws AppSearchException { mReadWriteLock.writeLock().lock(); try { diff --git a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/FrameworkOptimizeStrategy.java b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/FrameworkOptimizeStrategy.java new file mode 100644 index 0000000000000..8ec30e1863062 --- /dev/null +++ b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/FrameworkOptimizeStrategy.java @@ -0,0 +1,44 @@ +/* + * Copyright 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.android.server.appsearch.external.localstorage; + +import android.annotation.NonNull; + +import com.android.internal.annotations.VisibleForTesting; + +import com.google.android.icing.proto.GetOptimizeInfoResultProto; + +/** + * An implementation of {@link OptimizeStrategy} will determine when to trigger {@link + * AppSearchImpl#optimize()} in Jetpack environment. + * + * @hide + */ +public class FrameworkOptimizeStrategy implements OptimizeStrategy { + + @VisibleForTesting static final int DOC_COUNT_OPTIMIZE_THRESHOLD = 100_000; + @VisibleForTesting static final int BYTES_OPTIMIZE_THRESHOLD = 1 * 1024 * 1024 * 1024; // 1GB + + @VisibleForTesting + static final long TIME_OPTIMIZE_THRESHOLD_MILLIS = 7 * 24 * 60 * 60 * 1000; // 1 week + + @Override + public boolean shouldOptimize(@NonNull GetOptimizeInfoResultProto optimizeInfo) { + return optimizeInfo.getOptimizableDocs() >= DOC_COUNT_OPTIMIZE_THRESHOLD + || optimizeInfo.getEstimatedOptimizableBytes() >= BYTES_OPTIMIZE_THRESHOLD + || optimizeInfo.getTimeSinceLastOptimizeMs() >= TIME_OPTIMIZE_THRESHOLD_MILLIS; + } +} diff --git a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/OptimizeStrategy.java b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/OptimizeStrategy.java new file mode 100644 index 0000000000000..6cb84bc64eb9d --- /dev/null +++ b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/OptimizeStrategy.java @@ -0,0 +1,39 @@ +/* + * Copyright 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.appsearch.external.localstorage; + +import android.annotation.NonNull; + +import com.google.android.icing.proto.GetOptimizeInfoResultProto; + +/** + * An interface class for implementing a strategy to determine when to trigger {@link + * AppSearchImpl#optimize()}. + * + * @hide + */ +public interface OptimizeStrategy { + + /** + * Determines whether {@link AppSearchImpl#optimize()} need to be triggered to release garbage + * resources in AppSearch base on the given information. + * + * @param optimizeInfo The proto object indicates the number of garbage resources in AppSearch. + * @return {@code true} if {@link AppSearchImpl#optimize()} need to be triggered. + */ + boolean shouldOptimize(@NonNull GetOptimizeInfoResultProto optimizeInfo); +} diff --git a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/stats/CallStats.java b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/stats/CallStats.java index ea5263aa9aa53..81fb418a6a0a5 100644 --- a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/stats/CallStats.java +++ b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/stats/CallStats.java @@ -18,6 +18,8 @@ package com.android.server.appsearch.external.localstorage.stats; import android.annotation.IntDef; import android.annotation.NonNull; +import android.annotation.Nullable; +import android.app.appsearch.AppSearchResult; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; @@ -29,9 +31,9 @@ import java.util.Objects; *

This class can set which stats to log for both batch and non-batch {@link * android.app.appsearch.AppSearchSession} calls. * - *

Some function calls like {@link android.app.appsearch.AppSearchSession#setSchema} have their - * own detailed stats class {@link placeholder}. However, {@link CallStats} can still be used along - * with the detailed stats class for easy aggregation/analysis with other function calls. + *

Some function calls may have their own detailed stats class like {@link PutDocumentStats}. + * However, {@link CallStats} can still be used along with the detailed stats class for easy + * aggregation/analysis with other function calls. * * @hide */ @@ -73,7 +75,16 @@ public class CallStats { public static final int CALL_TYPE_REMOVE_DOCUMENTS_BY_SEARCH = 13; public static final int CALL_TYPE_REMOVE_DOCUMENT_BY_SEARCH = 14; - @NonNull private final GeneralStats mGeneralStats; + @Nullable private final String mPackageName; + @Nullable private final String mDatabase; + /** + * The status code returned by {@link AppSearchResult#getResultCode()} for the call or internal + * state. + */ + @AppSearchResult.ResultCode private final int mStatusCode; + + private final int mTotalLatencyMillis; + @CallType private final int mCallType; private final int mEstimatedBinderLatencyMillis; private final int mNumOperationsSucceeded; @@ -81,17 +92,37 @@ public class CallStats { CallStats(@NonNull Builder builder) { Objects.requireNonNull(builder); - mGeneralStats = Objects.requireNonNull(builder.mGeneralStatsBuilder).build(); + mPackageName = builder.mPackageName; + mDatabase = builder.mDatabase; + mStatusCode = builder.mStatusCode; + mTotalLatencyMillis = builder.mTotalLatencyMillis; mCallType = builder.mCallType; mEstimatedBinderLatencyMillis = builder.mEstimatedBinderLatencyMillis; mNumOperationsSucceeded = builder.mNumOperationsSucceeded; mNumOperationsFailed = builder.mNumOperationsFailed; } - /** Returns general information for the call. */ - @NonNull - public GeneralStats getGeneralStats() { - return mGeneralStats; + /** Returns calling package name. */ + @Nullable + public String getPackageName() { + return mPackageName; + } + + /** Returns calling database name. */ + @Nullable + public String getDatabase() { + return mDatabase; + } + + /** Returns status code for this api call. */ + @AppSearchResult.ResultCode + public int getStatusCode() { + return mStatusCode; + } + + /** Returns total latency of this api call in millis. */ + public int getTotalLatencyMillis() { + return mTotalLatencyMillis; } /** Returns type of the call. */ @@ -137,23 +168,41 @@ public class CallStats { /** Builder for {@link CallStats}. */ public static class Builder { - @NonNull final GeneralStats.Builder mGeneralStatsBuilder; + @Nullable String mPackageName; + @Nullable String mDatabase; + @AppSearchResult.ResultCode int mStatusCode; + int mTotalLatencyMillis; @CallType int mCallType; int mEstimatedBinderLatencyMillis; int mNumOperationsSucceeded; int mNumOperationsFailed; - /** Builder takes {@link GeneralStats.Builder}. */ - public Builder(@NonNull String packageName, @NonNull String database) { - Objects.requireNonNull(packageName); - Objects.requireNonNull(database); - mGeneralStatsBuilder = new GeneralStats.Builder(packageName, database); + /** Sets the PackageName used by the session. */ + @NonNull + public Builder setPackageName(@NonNull String packageName) { + mPackageName = Objects.requireNonNull(packageName); + return this; } - /** Returns {@link GeneralStats.Builder}. */ + /** Sets the database used by the session. */ @NonNull - public GeneralStats.Builder getGeneralStatsBuilder() { - return mGeneralStatsBuilder; + public Builder setDatabase(@NonNull String database) { + mDatabase = Objects.requireNonNull(database); + return this; + } + + /** Sets the status code. */ + @NonNull + public Builder setStatusCode(@AppSearchResult.ResultCode int statusCode) { + mStatusCode = statusCode; + return this; + } + + /** Sets total latency in millis. */ + @NonNull + public Builder setTotalLatencyMillis(int totalLatencyMillis) { + mTotalLatencyMillis = totalLatencyMillis; + return this; } /** Sets type of the call. */ diff --git a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/stats/GeneralStats.java b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/stats/GeneralStats.java deleted file mode 100644 index 53c1ee3f675f8..0000000000000 --- a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/stats/GeneralStats.java +++ /dev/null @@ -1,122 +0,0 @@ -/* - * Copyright 2021 The Android Open Source Project - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package com.android.server.appsearch.external.localstorage.stats; - -import android.annotation.NonNull; -import android.app.appsearch.AppSearchResult; - -import java.util.Objects; - -/** - * A class for holding general logging information. - * - *

This class cannot be logged by {@link - * com.android.server.appsearch.external.localstorage.AppSearchLogger} directly. It is used for - * defining general logging information that is shared across different stats classes. - * - * @see PutDocumentStats - * @see CallStats - * @hide - */ -public final class GeneralStats { - /** Package name of the application. */ - @NonNull private final String mPackageName; - - /** Database name within AppSearch. */ - @NonNull private final String mDatabase; - - /** - * The status code returned by {@link AppSearchResult#getResultCode()} for the call or internal - * state. - */ - @AppSearchResult.ResultCode private final int mStatusCode; - - private final int mTotalLatencyMillis; - - GeneralStats(@NonNull Builder builder) { - Objects.requireNonNull(builder); - mPackageName = Objects.requireNonNull(builder.mPackageName); - mDatabase = Objects.requireNonNull(builder.mDatabase); - mStatusCode = builder.mStatusCode; - mTotalLatencyMillis = builder.mTotalLatencyMillis; - } - - /** Returns package name. */ - @NonNull - public String getPackageName() { - return mPackageName; - } - - /** Returns database name. */ - @NonNull - public String getDatabase() { - return mDatabase; - } - - /** Returns result code from {@link AppSearchResult#getResultCode()} */ - @AppSearchResult.ResultCode - public int getStatusCode() { - return mStatusCode; - } - - /** Returns total latency, in milliseconds. */ - public int getTotalLatencyMillis() { - return mTotalLatencyMillis; - } - - /** Builder for {@link GeneralStats}. */ - public static class Builder { - @NonNull final String mPackageName; - @NonNull final String mDatabase; - @AppSearchResult.ResultCode int mStatusCode = AppSearchResult.RESULT_UNKNOWN_ERROR; - int mTotalLatencyMillis; - - /** - * Constructor - * - * @param packageName name of the package logging stats - * @param database name of the database logging stats - */ - public Builder(@NonNull String packageName, @NonNull String database) { - mPackageName = Objects.requireNonNull(packageName); - mDatabase = Objects.requireNonNull(database); - } - - /** Sets status code returned from {@link AppSearchResult#getResultCode()} */ - @NonNull - public Builder setStatusCode(@AppSearchResult.ResultCode int statusCode) { - mStatusCode = statusCode; - return this; - } - - /** Sets total latency, in milliseconds. */ - @NonNull - public Builder setTotalLatencyMillis(int totalLatencyMillis) { - mTotalLatencyMillis = totalLatencyMillis; - return this; - } - - /** - * Creates a new {@link GeneralStats} object from the contents of this {@link Builder} - * instance. - */ - @NonNull - public GeneralStats build() { - return new GeneralStats(/* builder= */ this); - } - } -} diff --git a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/stats/PutDocumentStats.java b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/stats/PutDocumentStats.java index d031172d29c1b..7ba181668bfd5 100644 --- a/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/stats/PutDocumentStats.java +++ b/apex/appsearch/service/java/com/android/server/appsearch/external/localstorage/stats/PutDocumentStats.java @@ -17,6 +17,7 @@ package com.android.server.appsearch.external.localstorage.stats; import android.annotation.NonNull; +import android.app.appsearch.AppSearchResult; import java.util.Objects; @@ -27,8 +28,15 @@ import java.util.Objects; * @hide */ public final class PutDocumentStats { - /** {@link GeneralStats} holds the general stats. */ - @NonNull private final GeneralStats mGeneralStats; + @NonNull private final String mPackageName; + @NonNull private final String mDatabase; + /** + * The status code returned by {@link AppSearchResult#getResultCode()} for the call or internal + * state. + */ + @AppSearchResult.ResultCode private final int mStatusCode; + + private final int mTotalLatencyMillis; /** Time used to generate a document proto from a Bundle. */ private final int mGenerateDocumentProtoLatencyMillis; @@ -61,7 +69,10 @@ public final class PutDocumentStats { PutDocumentStats(@NonNull Builder builder) { Objects.requireNonNull(builder); - mGeneralStats = Objects.requireNonNull(builder.mGeneralStatsBuilder).build(); + mPackageName = builder.mPackageName; + mDatabase = builder.mDatabase; + mStatusCode = builder.mStatusCode; + mTotalLatencyMillis = builder.mTotalLatencyMillis; mGenerateDocumentProtoLatencyMillis = builder.mGenerateDocumentProtoLatencyMillis; mRewriteDocumentTypesLatencyMillis = builder.mRewriteDocumentTypesLatencyMillis; mNativeLatencyMillis = builder.mNativeLatencyMillis; @@ -73,10 +84,27 @@ public final class PutDocumentStats { mNativeExceededMaxNumTokens = builder.mNativeExceededMaxNumTokens; } - /** Returns the {@link GeneralStats} object attached to this instance. */ + /** Returns calling package name. */ @NonNull - public GeneralStats getGeneralStats() { - return mGeneralStats; + public String getPackageName() { + return mPackageName; + } + + /** Returns calling database name. */ + @NonNull + public String getDatabase() { + return mDatabase; + } + + /** Returns status code for this putDocument. */ + @AppSearchResult.ResultCode + public int getStatusCode() { + return mStatusCode; + } + + /** Returns total latency of this putDocument in millis. */ + public int getTotalLatencyMillis() { + return mTotalLatencyMillis; } /** Returns time spent on generating document proto, in milliseconds. */ @@ -129,7 +157,10 @@ public final class PutDocumentStats { /** Builder for {@link PutDocumentStats}. */ public static class Builder { - @NonNull final GeneralStats.Builder mGeneralStatsBuilder; + @NonNull final String mPackageName; + @NonNull final String mDatabase; + @AppSearchResult.ResultCode int mStatusCode; + int mTotalLatencyMillis; int mGenerateDocumentProtoLatencyMillis; int mRewriteDocumentTypesLatencyMillis; int mNativeLatencyMillis; @@ -140,17 +171,24 @@ public final class PutDocumentStats { int mNativeNumTokensIndexed; boolean mNativeExceededMaxNumTokens; - /** Builder takes {@link GeneralStats.Builder}. */ + /** Builder for {@link PutDocumentStats} */ public Builder(@NonNull String packageName, @NonNull String database) { - Objects.requireNonNull(packageName); - Objects.requireNonNull(database); - mGeneralStatsBuilder = new GeneralStats.Builder(packageName, database); + mPackageName = Objects.requireNonNull(packageName); + mDatabase = Objects.requireNonNull(database); } - /** Returns {@link GeneralStats.Builder}. */ + /** Sets the status code. */ @NonNull - public GeneralStats.Builder getGeneralStatsBuilder() { - return mGeneralStatsBuilder; + public Builder setStatusCode(@AppSearchResult.ResultCode int statusCode) { + mStatusCode = statusCode; + return this; + } + + /** Sets total latency in millis. */ + @NonNull + public Builder setTotalLatencyMillis(int totalLatencyMillis) { + mTotalLatencyMillis = totalLatencyMillis; + return this; } /** Sets how much time we spend for generating document proto, in milliseconds. */ diff --git a/apex/appsearch/service/java/com/android/server/appsearch/stats/PlatformLogger.java b/apex/appsearch/service/java/com/android/server/appsearch/stats/PlatformLogger.java index 5a0199f8d78f0..31fead5e63147 100644 --- a/apex/appsearch/service/java/com/android/server/appsearch/stats/PlatformLogger.java +++ b/apex/appsearch/service/java/com/android/server/appsearch/stats/PlatformLogger.java @@ -192,9 +192,8 @@ public final class PlatformLogger implements AppSearchLogger { @GuardedBy("mLock") private void logStatsImplLocked(@NonNull CallStats stats) { mLastPushTimeMillisLocked = SystemClock.elapsedRealtime(); - ExtraStats extraStats = createExtraStatsLocked(stats.getGeneralStats().getPackageName(), - stats.getCallType()); - String database = stats.getGeneralStats().getDatabase(); + ExtraStats extraStats = createExtraStatsLocked(stats.getPackageName(), stats.getCallType()); + String database = stats.getDatabase(); try { int hashCodeForDatabase = calculateHashCodeMd5(database); AppSearchStatsLog.write(AppSearchStatsLog.APP_SEARCH_CALL_STATS_REPORTED, @@ -202,8 +201,8 @@ public final class PlatformLogger implements AppSearchLogger { extraStats.mSkippedSampleCount, extraStats.mPackageUid, hashCodeForDatabase, - stats.getGeneralStats().getStatusCode(), - stats.getGeneralStats().getTotalLatencyMillis(), + stats.getStatusCode(), + stats.getTotalLatencyMillis(), stats.getCallType(), stats.getEstimatedBinderLatencyMillis(), stats.getNumOperationsSucceeded(), @@ -224,9 +223,9 @@ public final class PlatformLogger implements AppSearchLogger { @GuardedBy("mLock") private void logStatsImplLocked(@NonNull PutDocumentStats stats) { mLastPushTimeMillisLocked = SystemClock.elapsedRealtime(); - ExtraStats extraStats = createExtraStatsLocked(stats.getGeneralStats().getPackageName(), - CallStats.CALL_TYPE_PUT_DOCUMENT); - String database = stats.getGeneralStats().getDatabase(); + ExtraStats extraStats = createExtraStatsLocked( + stats.getPackageName(), CallStats.CALL_TYPE_PUT_DOCUMENT); + String database = stats.getDatabase(); try { int hashCodeForDatabase = calculateHashCodeMd5(database); AppSearchStatsLog.write(AppSearchStatsLog.APP_SEARCH_PUT_DOCUMENT_STATS_REPORTED, @@ -234,8 +233,8 @@ public final class PlatformLogger implements AppSearchLogger { extraStats.mSkippedSampleCount, extraStats.mPackageUid, hashCodeForDatabase, - stats.getGeneralStats().getStatusCode(), - stats.getGeneralStats().getTotalLatencyMillis(), + stats.getStatusCode(), + stats.getTotalLatencyMillis(), stats.getGenerateDocumentProtoLatencyMillis(), stats.getRewriteDocumentTypesLatencyMillis(), stats.getNativeLatencyMillis(), diff --git a/apex/appsearch/service/java/com/android/server/appsearch/visibilitystore/VisibilityStore.java b/apex/appsearch/service/java/com/android/server/appsearch/visibilitystore/VisibilityStore.java index 95ed36898c734..af09210b2c18c 100644 --- a/apex/appsearch/service/java/com/android/server/appsearch/visibilitystore/VisibilityStore.java +++ b/apex/appsearch/service/java/com/android/server/appsearch/visibilitystore/VisibilityStore.java @@ -66,8 +66,8 @@ import java.util.Set; * @hide */ public class VisibilityStore { - /** No-op user id that won't have any visibility settings. */ - public static final int NO_OP_USER_ID = -1; + /** No-op uid that won't have any visibility settings. */ + public static final int NO_OP_UID = -1; /** Version for the visibility schema */ private static final int SCHEMA_VERSION = 0; @@ -106,7 +106,7 @@ public class VisibilityStore { * @param userContext Context of the user that the call is being made as */ public VisibilityStore(@NonNull AppSearchImpl appSearchImpl, @NonNull Context userContext) { - mAppSearchImpl = appSearchImpl; + mAppSearchImpl = Objects.requireNonNull(appSearchImpl); mUserContext = Objects.requireNonNull(userContext); } diff --git a/apex/appsearch/synced_jetpack_changeid.txt b/apex/appsearch/synced_jetpack_changeid.txt index 395292df120a9..78d39cc6deacf 100644 --- a/apex/appsearch/synced_jetpack_changeid.txt +++ b/apex/appsearch/synced_jetpack_changeid.txt @@ -1 +1 @@ -c35ced970a63a6c7b1d17f9706160579540850d6 +31a54dba5bda4d0109ea91eb1ac047c937cbaae3 diff --git a/services/tests/servicestests/src/com/android/server/appsearch/AppSearchImplPlatformTest.java b/services/tests/servicestests/src/com/android/server/appsearch/AppSearchImplPlatformTest.java index 3bcbcbdbd2c87..8666fa6ac35a1 100644 --- a/services/tests/servicestests/src/com/android/server/appsearch/AppSearchImplPlatformTest.java +++ b/services/tests/servicestests/src/com/android/server/appsearch/AppSearchImplPlatformTest.java @@ -57,6 +57,11 @@ import java.util.Map; /** This tests AppSearchImpl when it's running with a platform-backed VisibilityStore. */ public class AppSearchImplPlatformTest { + /** + * Always trigger optimize in this class. OptimizeStrategy will be tested in its own test class. + */ + private static final OptimizeStrategy ALWAYS_OPTIMIZE = optimizeInfo -> true; + @Rule public TemporaryFolder mTemporaryFolder = new TemporaryFolder(); private final Map mMockPackageManagers = new ArrayMap<>(); private Context mContext; @@ -88,7 +93,8 @@ public class AppSearchImplPlatformTest { AppSearchImpl.create( mTemporaryFolder.newFolder(), mContext, - /*logger=*/ null); + /*logger=*/ null, + ALWAYS_OPTIMIZE); mGlobalQuerierUid = mContext.getPackageManager().getPackageUid(mContext.getPackageName(), /*flags=*/ 0); diff --git a/services/tests/servicestests/src/com/android/server/appsearch/VisibilityStoreTest.java b/services/tests/servicestests/src/com/android/server/appsearch/VisibilityStoreTest.java index 6ac4d138c9d1b..4faffc025e2fa 100644 --- a/services/tests/servicestests/src/com/android/server/appsearch/VisibilityStoreTest.java +++ b/services/tests/servicestests/src/com/android/server/appsearch/VisibilityStoreTest.java @@ -56,6 +56,11 @@ import java.util.Collections; import java.util.Map; public class VisibilityStoreTest { + /** + * Always trigger optimize in this class. OptimizeStrategy will be tested in its own test class. + */ + private static final OptimizeStrategy ALWAYS_OPTIMIZE = optimizeInfo -> true; + @Rule public TemporaryFolder mTemporaryFolder = new TemporaryFolder(); private final Map mMockPackageManagers = new ArrayMap<>(); private Context mContext; @@ -87,7 +92,8 @@ public class VisibilityStoreTest { AppSearchImpl.create( mTemporaryFolder.newFolder(), mContext, - /*logger=*/ null); + /*logger=*/ null, + ALWAYS_OPTIMIZE); mUid = mContext.getPackageManager().getPackageUid(mContext.getPackageName(), /*flags=*/ 0); mVisibilityStore = appSearchImpl.getVisibilityStoreLocked(); diff --git a/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/AppSearchImplTest.java b/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/AppSearchImplTest.java index 5a8c44caa0a37..e26cfeab15295 100644 --- a/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/AppSearchImplTest.java +++ b/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/AppSearchImplTest.java @@ -76,17 +76,22 @@ import java.util.Set; public class AppSearchImplTest { @Rule public TemporaryFolder mTemporaryFolder = new TemporaryFolder(); private AppSearchImpl mAppSearchImpl; + /** + * Always trigger optimize in this class. OptimizeStrategy will be tested in its own test class. + */ + private static final OptimizeStrategy ALWAYS_OPTIMIZE = optimizeInfo -> true; @Before public void setUp() throws Exception { Context context = ApplicationProvider.getApplicationContext(); - // Give ourselves global query permissions + // Give ourselves global query permissions. mAppSearchImpl = AppSearchImpl.create( mTemporaryFolder.newFolder(), context, - /*logger=*/ null); + /*logger=*/ null, + ALWAYS_OPTIMIZE); } /** @@ -423,7 +428,7 @@ public class AppSearchImplTest { } @Test - public void testOptimize() throws Exception { + public void testTriggerCheckOptimizeByMutationSize() throws Exception { // Insert schema List schemas = Collections.singletonList(new AppSearchSchema.Builder("type").build()); @@ -436,56 +441,30 @@ public class AppSearchImplTest { /*forceOverride=*/ false, /*version=*/ 0); - // Insert enough documents. - for (int i = 0; - i - < AppSearchImpl.OPTIMIZE_THRESHOLD_DOC_COUNT - + AppSearchImpl.CHECK_OPTIMIZE_INTERVAL; - i++) { - GenericDocument document = - new GenericDocument.Builder<>("namespace", "id" + i, "type").build(); - mAppSearchImpl.putDocument("package", "database", document, /*logger=*/ null); - } + // Insert a document and then remove it to generate garbage. + GenericDocument document = new GenericDocument.Builder<>("namespace", "id", "type").build(); + mAppSearchImpl.putDocument("package", "database", document, /*logger=*/ null); + mAppSearchImpl.remove( + "package", "database", "namespace", "id", /*removeStatsBuilder=*/ null); - // Check optimize() will release 0 docs since there is no deletion. + // Verify there is garbage documents. GetOptimizeInfoResultProto optimizeInfo = mAppSearchImpl.getOptimizeInfoResultLocked(); + assertThat(optimizeInfo.getOptimizableDocs()).isEqualTo(1); + + // Increase mutation counter and stop before reach the threshold + mAppSearchImpl.checkForOptimize(AppSearchImpl.CHECK_OPTIMIZE_INTERVAL - 1); + + // Verify the optimize() isn't triggered. + optimizeInfo = mAppSearchImpl.getOptimizeInfoResultLocked(); + assertThat(optimizeInfo.getOptimizableDocs()).isEqualTo(1); + + // Increase the counter and reach the threshold, optimize() should be triggered. + mAppSearchImpl.checkForOptimize(/*mutateBatchSize=*/ 1); + + // Verify optimize() is triggered. + optimizeInfo = mAppSearchImpl.getOptimizeInfoResultLocked(); assertThat(optimizeInfo.getOptimizableDocs()).isEqualTo(0); - - // delete 999 documents, we will reach the threshold to trigger optimize() in next - // deletion. - for (int i = 0; i < AppSearchImpl.OPTIMIZE_THRESHOLD_DOC_COUNT - 1; i++) { - mAppSearchImpl.remove( - "package", "database", "namespace", "id" + i, /*removeStatsBuilder=*/ null); - } - - // Updates the check for optimize counter, checkForOptimize() will be triggered since - // CHECK_OPTIMIZE_INTERVAL is reached but optimize() won't since - // OPTIMIZE_THRESHOLD_DOC_COUNT is not. - mAppSearchImpl.checkForOptimize( - /*mutateBatchSize=*/ AppSearchImpl.OPTIMIZE_THRESHOLD_DOC_COUNT - 1); - - // Verify optimize() still not be triggered. - optimizeInfo = mAppSearchImpl.getOptimizeInfoResultLocked(); - assertThat(optimizeInfo.getOptimizableDocs()) - .isEqualTo(AppSearchImpl.OPTIMIZE_THRESHOLD_DOC_COUNT - 1); - - // Keep delete docs - for (int i = AppSearchImpl.OPTIMIZE_THRESHOLD_DOC_COUNT; - i - < AppSearchImpl.OPTIMIZE_THRESHOLD_DOC_COUNT - + AppSearchImpl.CHECK_OPTIMIZE_INTERVAL; - i++) { - mAppSearchImpl.remove( - "package", "database", "namespace", "id" + i, /*removeStatsBuilder=*/ null); - } - // updates the check for optimize counter, will reach both CHECK_OPTIMIZE_INTERVAL and - // OPTIMIZE_THRESHOLD_DOC_COUNT this time and trigger a optimize(). - mAppSearchImpl.checkForOptimize(/*mutateBatchSize*/ AppSearchImpl.CHECK_OPTIMIZE_INTERVAL); - - // Verify optimize() is triggered - optimizeInfo = mAppSearchImpl.getOptimizeInfoResultLocked(); - assertThat(optimizeInfo.getOptimizableDocs()) - .isLessThan(AppSearchImpl.CHECK_OPTIMIZE_INTERVAL); + assertThat(optimizeInfo.getEstimatedOptimizableBytes()).isEqualTo(0); } @Test @@ -493,7 +472,12 @@ public class AppSearchImplTest { // Setup the index Context context = ApplicationProvider.getApplicationContext(); File appsearchDir = mTemporaryFolder.newFolder(); - AppSearchImpl appSearchImpl = AppSearchImpl.create(appsearchDir, context, /*logger=*/ null); + AppSearchImpl appSearchImpl = + AppSearchImpl.create( + appsearchDir, + context, + /*logger=*/ null, + ALWAYS_OPTIMIZE); // Insert schema List schemas = @@ -522,7 +506,7 @@ public class AppSearchImplTest { /*queryExpression=*/ "", new SearchSpec.Builder().addFilterSchemas("Type1").build(), context.getPackageName(), - VisibilityStore.NO_OP_USER_ID, + VisibilityStore.NO_OP_UID, /*logger=*/ null); assertThat(results.getResults()).hasSize(1); assertThat(results.getResults().get(0).getGenericDocument()).isEqualTo(validDoc); @@ -554,7 +538,9 @@ public class AppSearchImplTest { // Initialize AppSearchImpl. This should cause a reset. appSearchImpl.close(); - appSearchImpl = AppSearchImpl.create(appsearchDir, context, testLogger); + appSearchImpl = + AppSearchImpl.create( + appsearchDir, context, testLogger, ALWAYS_OPTIMIZE); // Check recovery state InitializeStats initStats = testLogger.mInitializeStats; @@ -582,7 +568,7 @@ public class AppSearchImplTest { /*queryExpression=*/ "", new SearchSpec.Builder().addFilterSchemas("Type1").build(), context.getPackageName(), - VisibilityStore.NO_OP_USER_ID, + VisibilityStore.NO_OP_UID, /*logger=*/ null); assertThat(results.getResults()).isEmpty(); @@ -606,7 +592,7 @@ public class AppSearchImplTest { /*queryExpression=*/ "", new SearchSpec.Builder().addFilterSchemas("Type1").build(), context.getPackageName(), - VisibilityStore.NO_OP_USER_ID, + VisibilityStore.NO_OP_UID, /*logger=*/ null); assertThat(results.getResults()).hasSize(1); assertThat(results.getResults().get(0).getGenericDocument()).isEqualTo(validDoc); @@ -862,7 +848,7 @@ public class AppSearchImplTest { "", searchSpec, /*callerPackageName=*/ "", - /*callerUid=*/ 0, + VisibilityStore.NO_OP_UID, /*logger=*/ null); assertThat(searchResultPage.getResults()).isEmpty(); } @@ -1683,7 +1669,8 @@ public class AppSearchImplTest { AppSearchImpl.create( mTemporaryFolder.newFolder(), context, - /*logger=*/ null); + /*logger=*/ null, + ALWAYS_OPTIMIZE); // Initial check that we could do something at first. List schemas = @@ -1758,7 +1745,7 @@ public class AppSearchImplTest { .setTermMatch(TermMatchType.Code.PREFIX_VALUE) .build(), "package", - /*callerUid=*/ 1, + VisibilityStore.NO_OP_UID, /*logger=*/ null); }); @@ -1830,7 +1817,12 @@ public class AppSearchImplTest { // Setup the index Context context = ApplicationProvider.getApplicationContext(); File appsearchDir = mTemporaryFolder.newFolder(); - AppSearchImpl appSearchImpl = AppSearchImpl.create(appsearchDir, context, /*logger=*/ null); + AppSearchImpl appSearchImpl = + AppSearchImpl.create( + appsearchDir, + context, + /*logger=*/ null, + ALWAYS_OPTIMIZE); List schemas = Collections.singletonList(new AppSearchSchema.Builder("type").build()); @@ -1856,7 +1848,11 @@ public class AppSearchImplTest { // That document should be visible even from another instance. AppSearchImpl appSearchImpl2 = - AppSearchImpl.create(appsearchDir, context, /*logger=*/ null); + AppSearchImpl.create( + appsearchDir, + context, + /*logger=*/ null, + ALWAYS_OPTIMIZE); getResult = appSearchImpl2.getDocument( "package", "database", "namespace1", "id1", Collections.emptyMap()); @@ -1868,7 +1864,12 @@ public class AppSearchImplTest { // Setup the index Context context = ApplicationProvider.getApplicationContext(); File appsearchDir = mTemporaryFolder.newFolder(); - AppSearchImpl appSearchImpl = AppSearchImpl.create(appsearchDir, context, /*logger=*/ null); + AppSearchImpl appSearchImpl = + AppSearchImpl.create( + appsearchDir, + context, + /*logger=*/ null, + ALWAYS_OPTIMIZE); List schemas = Collections.singletonList(new AppSearchSchema.Builder("type").build()); @@ -1918,7 +1919,11 @@ public class AppSearchImplTest { // Only the second document should be retrievable from another instance. AppSearchImpl appSearchImpl2 = - AppSearchImpl.create(appsearchDir, context, /*logger=*/ null); + AppSearchImpl.create( + appsearchDir, + context, + /*logger=*/ null, + ALWAYS_OPTIMIZE); expectThrows( AppSearchException.class, () -> @@ -1939,7 +1944,12 @@ public class AppSearchImplTest { // Setup the index Context context = ApplicationProvider.getApplicationContext(); File appsearchDir = mTemporaryFolder.newFolder(); - AppSearchImpl appSearchImpl = AppSearchImpl.create(appsearchDir, context, /*logger=*/ null); + AppSearchImpl appSearchImpl = + AppSearchImpl.create( + appsearchDir, + context, + /*logger=*/ null, + ALWAYS_OPTIMIZE); List schemas = Collections.singletonList(new AppSearchSchema.Builder("type").build()); @@ -1997,7 +2007,11 @@ public class AppSearchImplTest { // Only the second document should be retrievable from another instance. AppSearchImpl appSearchImpl2 = - AppSearchImpl.create(appsearchDir, context, /*logger=*/ null); + AppSearchImpl.create( + appsearchDir, + context, + /*logger=*/ null, + ALWAYS_OPTIMIZE); expectThrows( AppSearchException.class, () -> diff --git a/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/AppSearchLoggerTest.java b/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/AppSearchLoggerTest.java index f0a6ef13b212a..c6726c69c7fd2 100644 --- a/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/AppSearchLoggerTest.java +++ b/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/AppSearchLoggerTest.java @@ -53,6 +53,10 @@ public class AppSearchLoggerTest { @Rule public TemporaryFolder mTemporaryFolder = new TemporaryFolder(); private AppSearchImpl mAppSearchImpl; private TestLogger mLogger; + /** + * Always trigger optimize in this class. OptimizeStrategy will be tested in its own test class. + */ + private static final OptimizeStrategy ALWAYS_OPTIMIZE = optimizeInfo -> true; @Before public void setUp() throws Exception { @@ -63,7 +67,8 @@ public class AppSearchLoggerTest { AppSearchImpl.create( mTemporaryFolder.newFolder(), context, - /*logger=*/ null); + /*logger=*/ null, + ALWAYS_OPTIMIZE); mLogger = new TestLogger(); } @@ -286,11 +291,13 @@ public class AppSearchLoggerTest { public void testLoggingStats_initialize() throws Exception { Context context = ApplicationProvider.getApplicationContext(); + // Create an unused AppSearchImpl to generated an InitializeStats. AppSearchImpl appSearchImpl = AppSearchImpl.create( mTemporaryFolder.newFolder(), context, - mLogger); + mLogger, + ALWAYS_OPTIMIZE); InitializeStats iStats = mLogger.mInitializeStats; assertThat(iStats).isNotNull(); @@ -325,9 +332,9 @@ public class AppSearchLoggerTest { PutDocumentStats pStats = mLogger.mPutDocumentStats; assertThat(pStats).isNotNull(); - assertThat(pStats.getGeneralStats().getPackageName()).isEqualTo(testPackageName); - assertThat(pStats.getGeneralStats().getDatabase()).isEqualTo(testDatabase); - assertThat(pStats.getGeneralStats().getStatusCode()).isEqualTo(AppSearchResult.RESULT_OK); + assertThat(pStats.getPackageName()).isEqualTo(testPackageName); + assertThat(pStats.getDatabase()).isEqualTo(testDatabase); + assertThat(pStats.getStatusCode()).isEqualTo(AppSearchResult.RESULT_OK); // The rest of native stats have been tested in testCopyNativeStats assertThat(pStats.getNativeDocumentSizeBytes()).isGreaterThan(0); } diff --git a/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/FrameworkOptimizeStrategyTest.java b/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/FrameworkOptimizeStrategyTest.java new file mode 100644 index 0000000000000..f30cbb8c51583 --- /dev/null +++ b/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/FrameworkOptimizeStrategyTest.java @@ -0,0 +1,68 @@ +/* + * Copyright 2021 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package com.android.server.appsearch.external.localstorage; + +import static com.android.server.appsearch.external.localstorage.FrameworkOptimizeStrategy.BYTES_OPTIMIZE_THRESHOLD; +import static com.android.server.appsearch.external.localstorage.FrameworkOptimizeStrategy.DOC_COUNT_OPTIMIZE_THRESHOLD; +import static com.android.server.appsearch.external.localstorage.FrameworkOptimizeStrategy.TIME_OPTIMIZE_THRESHOLD_MILLIS; + +import static com.google.common.truth.Truth.assertThat; + +import com.android.server.appsearch.proto.GetOptimizeInfoResultProto; +import com.android.server.appsearch.proto.StatusProto; + +import org.junit.Test; + +public class FrameworkOptimizeStrategyTest { + FrameworkOptimizeStrategy mFrameworkOptimizeStrategy = new FrameworkOptimizeStrategy(); + + @Test + public void testShouldOptimize_docCountThreshold() { + GetOptimizeInfoResultProto optimizeInfo = + GetOptimizeInfoResultProto.newBuilder() + .setTimeSinceLastOptimizeMs(0) + .setEstimatedOptimizableBytes(BYTES_OPTIMIZE_THRESHOLD) + .setOptimizableDocs(0) + .setStatus(StatusProto.newBuilder().setCode(StatusProto.Code.OK).build()) + .build(); + assertThat(mFrameworkOptimizeStrategy.shouldOptimize(optimizeInfo)).isTrue(); + } + + @Test + public void testShouldOptimize_byteThreshold() { + GetOptimizeInfoResultProto optimizeInfo = + GetOptimizeInfoResultProto.newBuilder() + .setTimeSinceLastOptimizeMs(TIME_OPTIMIZE_THRESHOLD_MILLIS) + .setEstimatedOptimizableBytes(0) + .setOptimizableDocs(0) + .setStatus(StatusProto.newBuilder().setCode(StatusProto.Code.OK).build()) + .build(); + assertThat(mFrameworkOptimizeStrategy.shouldOptimize(optimizeInfo)).isTrue(); + } + + @Test + public void testShouldNotOptimize_timeThreshold() { + GetOptimizeInfoResultProto optimizeInfo = + GetOptimizeInfoResultProto.newBuilder() + .setTimeSinceLastOptimizeMs(0) + .setEstimatedOptimizableBytes(0) + .setOptimizableDocs(DOC_COUNT_OPTIMIZE_THRESHOLD) + .setStatus(StatusProto.newBuilder().setCode(StatusProto.Code.OK).build()) + .build(); + assertThat(mFrameworkOptimizeStrategy.shouldOptimize(optimizeInfo)).isTrue(); + } +} diff --git a/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/stats/AppSearchStatsTest.java b/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/stats/AppSearchStatsTest.java index a71e532338481..6d9068675a727 100644 --- a/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/stats/AppSearchStatsTest.java +++ b/services/tests/servicestests/src/com/android/server/appsearch/external/localstorage/stats/AppSearchStatsTest.java @@ -28,34 +28,6 @@ public class AppSearchStatsTest { static final int TEST_STATUS_CODE = AppSearchResult.RESULT_INTERNAL_ERROR; static final int TEST_TOTAL_LATENCY_MILLIS = 20; - @Test - public void testAppSearchStats_GeneralStats() { - final GeneralStats gStats = - new GeneralStats.Builder(TEST_PACKAGE_NAME, TEST_DATA_BASE) - .setStatusCode(TEST_STATUS_CODE) - .setTotalLatencyMillis(TEST_TOTAL_LATENCY_MILLIS) - .build(); - - assertThat(gStats.getPackageName()).isEqualTo(TEST_PACKAGE_NAME); - assertThat(gStats.getDatabase()).isEqualTo(TEST_DATA_BASE); - assertThat(gStats.getStatusCode()).isEqualTo(TEST_STATUS_CODE); - assertThat(gStats.getTotalLatencyMillis()).isEqualTo(TEST_TOTAL_LATENCY_MILLIS); - } - - /** Make sure status code is UNKNOWN if not set in {@link GeneralStats} */ - @Test - public void testAppSearchStats_GeneralStats_defaultStatsCode_Unknown() { - final GeneralStats gStats = - new GeneralStats.Builder(TEST_PACKAGE_NAME, TEST_DATA_BASE) - .setTotalLatencyMillis(TEST_TOTAL_LATENCY_MILLIS) - .build(); - - assertThat(gStats.getPackageName()).isEqualTo(TEST_PACKAGE_NAME); - assertThat(gStats.getDatabase()).isEqualTo(TEST_DATA_BASE); - assertThat(gStats.getStatusCode()).isEqualTo(AppSearchResult.RESULT_UNKNOWN_ERROR); - assertThat(gStats.getTotalLatencyMillis()).isEqualTo(TEST_TOTAL_LATENCY_MILLIS); - } - @Test public void testAppSearchStats_CallStats() { final int estimatedBinderLatencyMillis = 1; @@ -63,23 +35,22 @@ public class AppSearchStatsTest { final int numOperationsFailed = 3; final @CallStats.CallType int callType = CallStats.CALL_TYPE_PUT_DOCUMENTS; - final CallStats.Builder cStatsBuilder = - new CallStats.Builder(TEST_PACKAGE_NAME, TEST_DATA_BASE) + final CallStats cStats = + new CallStats.Builder() + .setPackageName(TEST_PACKAGE_NAME) + .setDatabase(TEST_DATA_BASE) + .setStatusCode(TEST_STATUS_CODE) + .setTotalLatencyMillis(TEST_TOTAL_LATENCY_MILLIS) .setCallType(callType) .setEstimatedBinderLatencyMillis(estimatedBinderLatencyMillis) .setNumOperationsSucceeded(numOperationsSucceeded) - .setNumOperationsFailed(numOperationsFailed); - cStatsBuilder - .getGeneralStatsBuilder() - .setStatusCode(TEST_STATUS_CODE) - .setTotalLatencyMillis(TEST_TOTAL_LATENCY_MILLIS); - final CallStats cStats = cStatsBuilder.build(); + .setNumOperationsFailed(numOperationsFailed) + .build(); - assertThat(cStats.getGeneralStats().getPackageName()).isEqualTo(TEST_PACKAGE_NAME); - assertThat(cStats.getGeneralStats().getDatabase()).isEqualTo(TEST_DATA_BASE); - assertThat(cStats.getGeneralStats().getStatusCode()).isEqualTo(TEST_STATUS_CODE); - assertThat(cStats.getGeneralStats().getTotalLatencyMillis()) - .isEqualTo(TEST_TOTAL_LATENCY_MILLIS); + assertThat(cStats.getPackageName()).isEqualTo(TEST_PACKAGE_NAME); + assertThat(cStats.getDatabase()).isEqualTo(TEST_DATA_BASE); + assertThat(cStats.getStatusCode()).isEqualTo(TEST_STATUS_CODE); + assertThat(cStats.getTotalLatencyMillis()).isEqualTo(TEST_TOTAL_LATENCY_MILLIS); assertThat(cStats.getEstimatedBinderLatencyMillis()) .isEqualTo(estimatedBinderLatencyMillis); assertThat(cStats.getCallType()).isEqualTo(callType); @@ -87,6 +58,19 @@ public class AppSearchStatsTest { assertThat(cStats.getNumOperationsFailed()).isEqualTo(numOperationsFailed); } + @Test + public void testAppSearchCallStats_nullValues() { + final @CallStats.CallType int callType = CallStats.CALL_TYPE_PUT_DOCUMENTS; + + final CallStats.Builder cStatsBuilder = new CallStats.Builder().setCallType(callType); + + final CallStats cStats = cStatsBuilder.build(); + + assertThat(cStats.getPackageName()).isNull(); + assertThat(cStats.getDatabase()).isNull(); + assertThat(cStats.getCallType()).isEqualTo(callType); + } + @Test public void testAppSearchStats_PutDocumentStats() { final int generateDocumentProtoLatencyMillis = 1; @@ -100,6 +84,8 @@ public class AppSearchStatsTest { final boolean nativeExceededMaxNumTokens = true; final PutDocumentStats.Builder pStatsBuilder = new PutDocumentStats.Builder(TEST_PACKAGE_NAME, TEST_DATA_BASE) + .setStatusCode(TEST_STATUS_CODE) + .setTotalLatencyMillis(TEST_TOTAL_LATENCY_MILLIS) .setGenerateDocumentProtoLatencyMillis(generateDocumentProtoLatencyMillis) .setRewriteDocumentTypesLatencyMillis(rewriteDocumentTypesLatencyMillis) .setNativeLatencyMillis(nativeLatencyMillis) @@ -109,17 +95,13 @@ public class AppSearchStatsTest { .setNativeDocumentSizeBytes(nativeDocumentSize) .setNativeNumTokensIndexed(nativeNumTokensIndexed) .setNativeExceededMaxNumTokens(nativeExceededMaxNumTokens); - pStatsBuilder - .getGeneralStatsBuilder() - .setStatusCode(TEST_STATUS_CODE) - .setTotalLatencyMillis(TEST_TOTAL_LATENCY_MILLIS); + final PutDocumentStats pStats = pStatsBuilder.build(); - assertThat(pStats.getGeneralStats().getPackageName()).isEqualTo(TEST_PACKAGE_NAME); - assertThat(pStats.getGeneralStats().getDatabase()).isEqualTo(TEST_DATA_BASE); - assertThat(pStats.getGeneralStats().getStatusCode()).isEqualTo(TEST_STATUS_CODE); - assertThat(pStats.getGeneralStats().getTotalLatencyMillis()) - .isEqualTo(TEST_TOTAL_LATENCY_MILLIS); + assertThat(pStats.getPackageName()).isEqualTo(TEST_PACKAGE_NAME); + assertThat(pStats.getDatabase()).isEqualTo(TEST_DATA_BASE); + assertThat(pStats.getStatusCode()).isEqualTo(TEST_STATUS_CODE); + assertThat(pStats.getTotalLatencyMillis()).isEqualTo(TEST_TOTAL_LATENCY_MILLIS); assertThat(pStats.getGenerateDocumentProtoLatencyMillis()) .isEqualTo(generateDocumentProtoLatencyMillis); assertThat(pStats.getRewriteDocumentTypesLatencyMillis())