diff options
-rw-r--r-- | artd/artd.cc | 28 | ||||
-rw-r--r-- | artd/artd.h | 6 | ||||
-rw-r--r-- | artd/artd_test.cc | 131 | ||||
-rw-r--r-- | artd/binder/com/android/server/art/ArtConstants.aidl | 32 | ||||
-rw-r--r-- | artd/binder/com/android/server/art/IArtd.aidl | 14 | ||||
-rw-r--r-- | artd/file_utils.cc | 3 | ||||
-rw-r--r-- | libartservice/service/java/com/android/server/art/ArtManagerLocal.java | 106 | ||||
-rw-r--r-- | libartservice/service/java/com/android/server/art/ArtShellCommand.java | 6 | ||||
-rw-r--r-- | libartservice/service/java/com/android/server/art/BackgroundDexoptJob.java | 11 | ||||
-rw-r--r-- | libartservice/service/java/com/android/server/art/Utils.java | 9 | ||||
-rw-r--r-- | libartservice/service/java/com/android/server/art/model/DexoptParams.java | 5 | ||||
-rw-r--r-- | libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java | 63 | ||||
-rw-r--r-- | libartservice/service/javatests/com/android/server/art/BackgroundDexoptJobTest.java | 2 | ||||
-rw-r--r-- | libartservice/service/javatests/com/android/server/art/model/DexoptParamsTest.java | 5 | ||||
-rw-r--r-- | runtime/oat_file.cc | 2 | ||||
-rw-r--r-- | runtime/oat_file.h | 4 |
16 files changed, 419 insertions, 8 deletions
diff --git a/artd/artd.cc b/artd/artd.cc index 5b91879cfc..ffa30f4811 100644 --- a/artd/artd.cc +++ b/artd/artd.cc @@ -37,6 +37,7 @@ #include <string_view> #include <system_error> #include <type_traits> +#include <unordered_set> #include <utility> #include <vector> @@ -1012,6 +1013,33 @@ ScopedAStatus Artd::createCancellationSignal( return ScopedAStatus::ok(); } +ScopedAStatus Artd::cleanup(const std::vector<ProfilePath>& in_profilesToKeep, + const std::vector<ArtifactsPath>& in_artifactsToKeep, + const std::vector<VdexPath>& in_vdexFilesToKeep, + int64_t* _aidl_return) { + std::unordered_set<std::string> files_to_keep; + for (const ProfilePath& profile : in_profilesToKeep) { + files_to_keep.insert(OR_RETURN_FATAL(BuildProfileOrDmPath(profile))); + } + for (const ArtifactsPath& artifacts : in_artifactsToKeep) { + std::string oat_path = OR_RETURN_FATAL(BuildOatPath(artifacts)); + files_to_keep.insert(OatPathToVdexPath(oat_path)); + files_to_keep.insert(OatPathToArtPath(oat_path)); + files_to_keep.insert(std::move(oat_path)); + } + for (const VdexPath& vdex : in_vdexFilesToKeep) { + files_to_keep.insert(OR_RETURN_FATAL(BuildVdexPath(vdex))); + } + *_aidl_return = 0; + for (const std::string& file : OR_RETURN_NON_FATAL(ListManagedFiles())) { + if (files_to_keep.find(file) == files_to_keep.end()) { + LOG(INFO) << "Cleaning up obsolete file '{}'"_format(file); + *_aidl_return += GetSizeAndDeleteFile(file); + } + } + return ScopedAStatus::ok(); +} + Result<void> Artd::Start() { ScopedAStatus status = ScopedAStatus::fromStatus( AServiceManager_registerLazyService(this->asBinder().get(), kServiceName)); diff --git a/artd/artd.h b/artd/artd.h index fbaaed0dbe..ea52752799 100644 --- a/artd/artd.h +++ b/artd/artd.h @@ -158,6 +158,12 @@ class Artd : public aidl::com::android::server::art::BnArtd { std::shared_ptr<aidl::com::android::server::art::IArtdCancellationSignal>* _aidl_return) override; + ndk::ScopedAStatus cleanup( + const std::vector<aidl::com::android::server::art::ProfilePath>& in_profilesToKeep, + const std::vector<aidl::com::android::server::art::ArtifactsPath>& in_artifactsToKeep, + const std::vector<aidl::com::android::server::art::VdexPath>& in_vdexFilesToKeep, + int64_t* _aidl_return) override; + android::base::Result<void> Start(); private: diff --git a/artd/artd_test.cc b/artd/artd_test.cc index 8d03536881..2f8588715c 100644 --- a/artd/artd_test.cc +++ b/artd/artd_test.cc @@ -37,6 +37,7 @@ #include <utility> #include <vector> +#include "aidl/com/android/server/art/ArtConstants.h" #include "aidl/com/android/server/art/BnArtd.h" #include "android-base/collections.h" #include "android-base/errors.h" @@ -54,6 +55,7 @@ #include "fmt/format.h" #include "gmock/gmock.h" #include "gtest/gtest.h" +#include "oat_file.h" #include "path_utils.h" #include "profman/profman_result.h" #include "testing.h" @@ -63,12 +65,14 @@ namespace art { namespace artd { namespace { +using ::aidl::com::android::server::art::ArtConstants; using ::aidl::com::android::server::art::ArtdDexoptResult; using ::aidl::com::android::server::art::ArtifactsPath; using ::aidl::com::android::server::art::DexMetadataPath; using ::aidl::com::android::server::art::DexoptOptions; using ::aidl::com::android::server::art::FileVisibility; using ::aidl::com::android::server::art::FsPermission; +using ::aidl::com::android::server::art::GetDexoptStatusResult; using ::aidl::com::android::server::art::IArtdCancellationSignal; using ::aidl::com::android::server::art::OutputArtifacts; using ::aidl::com::android::server::art::OutputProfile; @@ -331,6 +335,11 @@ class ArtdTest : public CommonArtTest { std::filesystem::create_directories(android_data_); setenv("ANDROID_DATA", android_data_.c_str(), /*overwrite=*/1); + // Use an arbitrary existing directory as Android expand. + android_expand_ = scratch_path_ + "/mnt/expand"; + std::filesystem::create_directories(android_expand_); + setenv("ANDROID_EXPAND", android_expand_.c_str(), /*overwrite=*/1); + dex_file_ = scratch_path_ + "/a/b.apk"; isa_ = "arm64"; artifacts_path_ = ArtifactsPath{ @@ -426,9 +435,12 @@ class ArtdTest : public CommonArtTest { std::string scratch_path_; std::string art_root_; std::string android_data_; + std::string android_expand_; MockFunction<android::base::LogFunction> mock_logger_; ScopedUnsetEnvironmentVariable art_root_env_ = ScopedUnsetEnvironmentVariable("ANDROID_ART_ROOT"); ScopedUnsetEnvironmentVariable android_data_env_ = ScopedUnsetEnvironmentVariable("ANDROID_DATA"); + ScopedUnsetEnvironmentVariable android_expand_env_ = + ScopedUnsetEnvironmentVariable("ANDROID_EXPAND"); MockSystemProperties* mock_props_; MockExecUtils* mock_exec_utils_; MockFunction<int(pid_t, int)> mock_kill_; @@ -483,6 +495,8 @@ class ArtdTest : public CommonArtTest { } }; +TEST_F(ArtdTest, ConstantsAreInSync) { EXPECT_EQ(ArtConstants::REASON_VDEX, kReasonVdex); } + TEST_F(ArtdTest, isAlive) { bool result = false; artd_->isAlive(&result); @@ -1817,6 +1831,123 @@ TEST_F(ArtdTest, mergeProfilesWithOptionsDumpClassesAndMethods) { CheckContent(output_profile.profilePath.tmpPath, "dump"); } +TEST_F(ArtdTest, cleanup) { + std::vector<std::string> gc_removed_files; + std::vector<std::string> gc_kept_files; + + auto CreateGcRemovedFile = [&](const std::string& path) { + CreateFile(path); + gc_removed_files.push_back(path); + }; + + auto CreateGcKeptFile = [&](const std::string& path) { + CreateFile(path); + gc_kept_files.push_back(path); + }; + + // Unmanaged files. + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/1.odex"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/oat/1.odex"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/oat/1.txt"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.txt"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.tmp"); + + // Files to keep. + CreateGcKeptFile(android_data_ + "/misc/profiles/cur/1/com.android.foo/primary.prof"); + CreateGcKeptFile(android_data_ + "/misc/profiles/cur/3/com.android.foo/primary.prof"); + CreateGcKeptFile(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.dex"); + CreateGcKeptFile(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.vdex"); + CreateGcKeptFile(android_data_ + "/dalvik-cache/arm64/system@app@Foo@Foo.apk@classes.art"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/1.vdex"); + CreateGcKeptFile( + android_expand_ + + "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/oat/arm64/base.odex"); + CreateGcKeptFile( + android_expand_ + + "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/oat/arm64/base.vdex"); + CreateGcKeptFile( + android_expand_ + + "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/oat/arm64/base.art"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/2.odex"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/2.vdex"); + CreateGcKeptFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/2.art"); + + // Files to remove. + CreateGcRemovedFile(android_data_ + "/misc/profiles/ref/com.android.foo/primary.prof"); + CreateGcRemovedFile(android_data_ + "/misc/profiles/cur/2/com.android.foo/primary.prof"); + CreateGcRemovedFile(android_data_ + "/misc/profiles/cur/3/com.android.bar/primary.prof"); + CreateGcRemovedFile(android_data_ + "/dalvik-cache/arm64/extra.odex"); + CreateGcRemovedFile(android_data_ + "/dalvik-cache/arm64/system@app@Bar@Bar.apk@classes.dex"); + CreateGcRemovedFile(android_data_ + "/dalvik-cache/arm64/system@app@Bar@Bar.apk@classes.vdex"); + CreateGcRemovedFile(android_data_ + "/dalvik-cache/arm64/system@app@Bar@Bar.apk@classes.art"); + CreateGcRemovedFile( + android_expand_ + + "/123456-7890/app/~~daewfweaf==/com.android.foo-fjuwidhia==/oat/arm64/base.odex"); + CreateGcRemovedFile( + android_expand_ + + "/123456-7890/app/~~daewfweaf==/com.android.foo-fjuwidhia==/oat/arm64/base.vdex"); + CreateGcRemovedFile( + android_expand_ + + "/123456-7890/app/~~daewfweaf==/com.android.foo-fjuwidhia==/oat/arm64/base.art"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/1.prof"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/1.prof.123456.tmp"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.odex"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.vdex"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.art"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/1.odex.123456.tmp"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/oat/arm64/2.odex.123456.tmp"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/1.odex"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/1.art"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/oat/arm64/1.vdex.123456.tmp"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/bbb/oat/arm64/1.odex"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/bbb/oat/arm64/1.vdex"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.foo/aaa/bbb/oat/arm64/1.art"); + CreateGcRemovedFile(android_data_ + + "/user_de/0/com.android.foo/aaa/bbb/oat/arm64/1.art.123456.tmp"); + CreateGcRemovedFile(android_data_ + "/user_de/0/com.android.bar/aaa/oat/arm64/1.vdex"); + + int64_t aidl_return; + ASSERT_TRUE( + artd_ + ->cleanup( + { + PrimaryCurProfilePath{ + .userId = 1, .packageName = "com.android.foo", .profileName = "primary"}, + PrimaryCurProfilePath{ + .userId = 3, .packageName = "com.android.foo", .profileName = "primary"}, + }, + { + ArtifactsPath{.dexPath = "/system/app/Foo/Foo.apk", + .isa = "arm64", + .isInDalvikCache = true}, + ArtifactsPath{ + .dexPath = + android_expand_ + + "/123456-7890/app/~~nkfeankfna==/com.android.bar-jfoeaofiew==/base.apk", + .isa = "arm64", + .isInDalvikCache = false}, + ArtifactsPath{.dexPath = android_data_ + "/user_de/0/com.android.foo/aaa/2.apk", + .isa = "arm64", + .isInDalvikCache = false}, + }, + { + VdexPath{ArtifactsPath{ + .dexPath = android_data_ + "/user_de/0/com.android.foo/aaa/1.apk", + .isa = "arm64", + .isInDalvikCache = false}}, + }, + &aidl_return) + .isOk()); + + for (const std::string& path : gc_removed_files) { + EXPECT_FALSE(std::filesystem::exists(path)) << "'{}' should be removed"_format(path); + } + + for (const std::string& path : gc_kept_files) { + EXPECT_TRUE(std::filesystem::exists(path)) << "'{}' should be kept"_format(path); + } +} + } // namespace } // namespace artd } // namespace art diff --git a/artd/binder/com/android/server/art/ArtConstants.aidl b/artd/binder/com/android/server/art/ArtConstants.aidl new file mode 100644 index 0000000000..e9f702ef07 --- /dev/null +++ b/artd/binder/com/android/server/art/ArtConstants.aidl @@ -0,0 +1,32 @@ +/* + * Copyright (C) 2023 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.art; + +/** + * Constants used by ART Service Java code that must be kept in sync with those in ART native code. + * + * @hide + */ +parcelable ArtConstants { + /** + * A special compilation reason to indicate that only the VDEX file is usable. Keep in sync with + * {@code kReasonVdex} in art/runtime/oat_file.h. + * + * This isn't a valid reason to feed into DexoptParams. + */ + const @utf8InCpp String REASON_VDEX = "vdex"; +} diff --git a/artd/binder/com/android/server/art/IArtd.aidl b/artd/binder/com/android/server/art/IArtd.aidl index 5121063287..603a40f4cc 100644 --- a/artd/binder/com/android/server/art/IArtd.aidl +++ b/artd/binder/com/android/server/art/IArtd.aidl @@ -155,4 +155,18 @@ interface IArtd { * Returns a cancellation signal which can be used to cancel {@code dexopt} calls. */ com.android.server.art.IArtdCancellationSignal createCancellationSignal(); + + /** + * Deletes all files that are managed by artd, except those specified in the arguments. Returns + * the size of the freed space, in bytes. + * + * For each entry in `artifactsToKeep`, all three kinds of artifacts (ODEX, VDEX, ART) are + * kept. For each entry in `vdexFilesToKeep`, only the VDEX file will be kept. Note that VDEX + * files included in `artifactsToKeep` don't have to be listed in `vdexFilesToKeep`. + * + * Throws fatal errors. Logs and ignores non-fatal errors. + */ + long cleanup(in List<com.android.server.art.ProfilePath> profilesToKeep, + in List<com.android.server.art.ArtifactsPath> artifactsToKeep, + in List<com.android.server.art.VdexPath> vdexFilesToKeep); } diff --git a/artd/file_utils.cc b/artd/file_utils.cc index 53fe9f9625..0ce782a7e9 100644 --- a/artd/file_utils.cc +++ b/artd/file_utils.cc @@ -81,6 +81,9 @@ Result<void> NewFile::CommitOrAbandon() { std::error_code ec; std::filesystem::rename(temp_path_, final_path_, ec); if (ec) { + // If this fails because the temp file doesn't exist, it could be that the file is deleted by + // `Artd::cleanup` if that method is run simultaneously. At the time of writing, this should + // never happen because `Artd::cleanup` is only called at the end of the backgrond dexopt job. return Errorf( "Failed to move new file '{}' to path '{}': {}", temp_path_, final_path_, ec.message()); } diff --git a/libartservice/service/java/com/android/server/art/ArtManagerLocal.java b/libartservice/service/java/com/android/server/art/ArtManagerLocal.java index 07d77aa858..07e7598c2a 100644 --- a/libartservice/service/java/com/android/server/art/ArtManagerLocal.java +++ b/libartservice/service/java/com/android/server/art/ArtManagerLocal.java @@ -16,6 +16,7 @@ package com.android.server.art; +import static com.android.server.art.DexUseManagerLocal.DetailedSecondaryDexInfo; import static com.android.server.art.DexUseManagerLocal.SecondaryDexInfo; import static com.android.server.art.PrimaryDexUtils.DetailedPrimaryDexInfo; import static com.android.server.art.PrimaryDexUtils.PrimaryDexInfo; @@ -56,6 +57,7 @@ import com.android.server.art.model.ArtFlags; import com.android.server.art.model.BatchDexoptParams; import com.android.server.art.model.Config; import com.android.server.art.model.DeleteResult; +import com.android.server.art.model.DetailedDexInfo; import com.android.server.art.model.DexoptParams; import com.android.server.art.model.DexoptResult; import com.android.server.art.model.DexoptStatus; @@ -65,6 +67,8 @@ import com.android.server.pm.pkg.AndroidPackage; import com.android.server.pm.pkg.AndroidPackageSplit; import com.android.server.pm.pkg.PackageState; +import dalvik.system.DexFile; + import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; @@ -815,6 +819,108 @@ public final class ArtManagerLocal { } /** + * Cleans up obsolete profiles and artifacts. + * + * This is done in a mark-and-sweep approach. + * + * @hide + */ + public void cleanup(@NonNull PackageManagerLocal.FilteredSnapshot snapshot) { + try { + // For every primary dex container file or secondary dex container file of every app, if + // it has code, we keep the following types of files: + // - The reference profile and the current profiles, regardless of the hibernation state + // of the app. + // - The dexopt artifacts, if they are up-to-date and the app is not hibernating. + // - Only the VDEX part of the dexopt artifacts, if the dexopt artifacts are outdated + // but the VDEX part is still usable and the app is not hibernating. + List<ProfilePath> profilesToKeep = new ArrayList<>(); + List<ArtifactsPath> artifactsToKeep = new ArrayList<>(); + List<VdexPath> vdexFilesToKeep = new ArrayList<>(); + + for (PackageState pkgState : snapshot.getPackageStates().values()) { + if (!Utils.canDexoptPackage(pkgState, null /* appHibernationManager */)) { + continue; + } + AndroidPackage pkg = Utils.getPackageOrThrow(pkgState); + boolean isInDalvikCache = Utils.isInDalvikCache(pkgState); + boolean keepArtifacts = !Utils.shouldSkipDexoptDueToHibernation( + pkgState, mInjector.getAppHibernationManager()); + for (DetailedPrimaryDexInfo dexInfo : + PrimaryDexUtils.getDetailedDexInfo(pkgState, pkg)) { + if (!dexInfo.hasCode()) { + continue; + } + profilesToKeep.add(PrimaryDexUtils.buildRefProfilePath(pkgState, dexInfo)); + profilesToKeep.addAll(PrimaryDexUtils.getCurProfiles( + mInjector.getUserManager(), pkgState, dexInfo)); + if (keepArtifacts) { + for (Abi abi : Utils.getAllAbis(pkgState)) { + maybeKeepArtifacts(artifactsToKeep, vdexFilesToKeep, pkgState, dexInfo, + abi, isInDalvikCache); + } + } + } + for (DetailedSecondaryDexInfo dexInfo : + mInjector.getDexUseManager().getFilteredDetailedSecondaryDexInfo( + pkgState.getPackageName())) { + profilesToKeep.add( + AidlUtils.buildProfilePathForSecondaryRef(dexInfo.dexPath())); + profilesToKeep.add( + AidlUtils.buildProfilePathForSecondaryCur(dexInfo.dexPath())); + if (keepArtifacts) { + for (Abi abi : Utils.getAllAbisForNames(dexInfo.abiNames(), pkgState)) { + maybeKeepArtifacts(artifactsToKeep, vdexFilesToKeep, pkgState, dexInfo, + abi, false /* isInDalvikCache */); + } + } + } + } + long freedBytes = + mInjector.getArtd().cleanup(profilesToKeep, artifactsToKeep, vdexFilesToKeep); + Log.i(TAG, String.format("Freed %d bytes", freedBytes)); + } catch (RemoteException e) { + throw new IllegalStateException("An error occurred when calling artd", e); + } + } + + /** + * Checks if the artifacts are up-to-date, and maybe adds them to {@code artifactsToKeep} or + * {@code vdexFilesToKeep} based on the result. + */ + private void maybeKeepArtifacts(@NonNull List<ArtifactsPath> artifactsToKeep, + @NonNull List<VdexPath> vdexFilesToKeep, @NonNull PackageState pkgState, + @NonNull DetailedDexInfo dexInfo, @NonNull Abi abi, boolean isInDalvikCache) + throws RemoteException { + try { + GetDexoptStatusResult result = mInjector.getArtd().getDexoptStatus( + dexInfo.dexPath(), abi.isa(), dexInfo.classLoaderContext()); + if (DexFile.isValidCompilerFilter(result.compilerFilter)) { + // TODO(b/263579377): This is a bit inaccurate. We may be keeping the artifacts in + // dalvik-cache while OatFileAssistant actually picks the ones not in dalvik-cache. + // However, this isn't a big problem because it is an edge case and it only causes + // us to delete less rather than deleting more. + ArtifactsPath artifacts = + AidlUtils.buildArtifactsPath(dexInfo.dexPath(), abi.isa(), isInDalvikCache); + if (result.compilationReason.equals(ArtConstants.REASON_VDEX)) { + // Only the VDEX file is usable. + vdexFilesToKeep.add(VdexPath.artifactsPath(artifacts)); + } else { + artifactsToKeep.add(artifacts); + } + } + } catch (ServiceSpecificException e) { + // Don't add the artifacts to the lists. They should be cleaned up. + Log.e(TAG, + String.format("Failed to get dexopt status [packageName = %s, dexPath = %s, " + + "isa = %s, classLoaderContext = %s]", + pkgState.getPackageName(), dexInfo.dexPath(), abi.isa(), + dexInfo.classLoaderContext()), + e); + } + } + + /** * Should be used by {@link BackgroundDexoptJobService} ONLY. * * @hide diff --git a/libartservice/service/java/com/android/server/art/ArtShellCommand.java b/libartservice/service/java/com/android/server/art/ArtShellCommand.java index 6fd1dc563b..9ac966370c 100644 --- a/libartservice/service/java/com/android/server/art/ArtShellCommand.java +++ b/libartservice/service/java/com/android/server/art/ArtShellCommand.java @@ -352,6 +352,10 @@ public final class ArtShellCommand extends BasicShellCommandHandler { } return 0; } + case "cleanup": { + mArtManagerLocal.cleanup(snapshot); + return 0; + } default: pw.println(String.format("Unknown 'art' sub-command '%s'", subcmd)); pw.println("See 'cmd package help' for help"); @@ -453,6 +457,8 @@ public final class ArtShellCommand extends BasicShellCommandHandler { pw.println(" The profile of the base APK is dumped to 'PACKAGE_NAME-primary.prof.txt'"); pw.println(" The profile of a split APK is dumped to"); pw.println(" 'PACKAGE_NAME-SPLIT_NAME.split.prof.txt'"); + pw.println(" cleanup"); + pw.println(" Cleanup obsolete files."); } private void enforceRoot() { diff --git a/libartservice/service/java/com/android/server/art/BackgroundDexoptJob.java b/libartservice/service/java/com/android/server/art/BackgroundDexoptJob.java index e6ba579f18..92989282a0 100644 --- a/libartservice/service/java/com/android/server/art/BackgroundDexoptJob.java +++ b/libartservice/service/java/com/android/server/art/BackgroundDexoptJob.java @@ -196,13 +196,22 @@ public class BackgroundDexoptJob { @NonNull private CompletedResult run(@NonNull CancellationSignal cancellationSignal) { // TODO(b/254013427): Cleanup dex use info. - // TODO(b/254013425): Cleanup unused secondary dex file artifacts. long startTimeMs = SystemClock.uptimeMillis(); DexoptResult dexoptResult; try (var snapshot = mInjector.getPackageManagerLocal().withFilteredSnapshot()) { dexoptResult = mInjector.getArtManagerLocal().dexoptPackages(snapshot, ReasonMapping.REASON_BG_DEXOPT, cancellationSignal, null /* processCallbackExecutor */, null /* processCallback */); + + // For simplicity, we don't support cancelling the following operation in the middle. + // This is fine because it typically takes only a few seconds. + if (!cancellationSignal.isCanceled()) { + // We do the cleanup after dexopt so that it doesn't affect the `getSizeBeforeBytes` + // field in the result that we send to callbacks. Admittedly, this will cause us to + // lose some chance to dexopt when the storage is very low, but it's fine because we + // can still dexopt in the next run. + mInjector.getArtManagerLocal().cleanup(snapshot); + } } return CompletedResult.create(dexoptResult, SystemClock.uptimeMillis() - startTimeMs); } diff --git a/libartservice/service/java/com/android/server/art/Utils.java b/libartservice/service/java/com/android/server/art/Utils.java index 0d409231f2..5592850490 100644 --- a/libartservice/service/java/com/android/server/art/Utils.java +++ b/libartservice/service/java/com/android/server/art/Utils.java @@ -257,14 +257,19 @@ public final class Utils { // We do not dexopt unused packages. // If `appHibernationManager` is null, the caller's intention is to skip the check. if (appHibernationManager != null - && appHibernationManager.isHibernatingGlobally(pkgState.getPackageName()) - && appHibernationManager.isOatArtifactDeletionEnabled()) { + && shouldSkipDexoptDueToHibernation(pkgState, appHibernationManager)) { return false; } return true; } + public static boolean shouldSkipDexoptDueToHibernation( + @NonNull PackageState pkgState, @NonNull AppHibernationManager appHibernationManager) { + return appHibernationManager.isHibernatingGlobally(pkgState.getPackageName()) + && appHibernationManager.isOatArtifactDeletionEnabled(); + } + public static long getPackageLastActiveTime(@NonNull PackageState pkgState, @NonNull DexUseManagerLocal dexUseManager, @NonNull UserManager userManager) { long lastUsedAtMs = dexUseManager.getPackageLastUsedAtMs(pkgState.getPackageName()); diff --git a/libartservice/service/java/com/android/server/art/model/DexoptParams.java b/libartservice/service/java/com/android/server/art/model/DexoptParams.java index 4dc94714a8..f7ce89412f 100644 --- a/libartservice/service/java/com/android/server/art/model/DexoptParams.java +++ b/libartservice/service/java/com/android/server/art/model/DexoptParams.java @@ -24,6 +24,7 @@ import android.annotation.Nullable; import android.annotation.SystemApi; import com.android.internal.annotations.Immutable; +import com.android.server.art.ArtConstants; import com.android.server.art.ReasonMapping; import com.android.server.art.Utils; @@ -120,6 +121,10 @@ public class DexoptParams { if (mParams.mReason.isEmpty()) { throw new IllegalArgumentException("Reason must not be empty"); } + if (mParams.mReason.equals(ArtConstants.REASON_VDEX)) { + throw new IllegalArgumentException( + "Reason must not be '" + ArtConstants.REASON_VDEX + "'"); + } if (mParams.mCompilerFilter.isEmpty()) { mParams.mCompilerFilter = ReasonMapping.getCompilerFilterForReason(mParams.mReason); diff --git a/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java b/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java index 08db07fcb0..9f83c8605f 100644 --- a/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java +++ b/libartservice/service/javatests/com/android/server/art/ArtManagerLocalTest.java @@ -18,7 +18,7 @@ package com.android.server.art; import static android.os.ParcelFileDescriptor.AutoCloseInputStream; -import static com.android.server.art.DexUseManagerLocal.SecondaryDexInfo; +import static com.android.server.art.DexUseManagerLocal.DetailedSecondaryDexInfo; import static com.android.server.art.model.DexoptStatus.DexContainerFileDexoptStatus; import static com.android.server.art.testing.TestingUtils.deepEq; import static com.android.server.art.testing.TestingUtils.inAnyOrder; @@ -85,6 +85,7 @@ import java.io.IOException; import java.io.InputStream; import java.nio.charset.StandardCharsets; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.concurrent.ForkJoinPool; import java.util.concurrent.TimeUnit; @@ -190,8 +191,12 @@ public class ArtManagerLocalTest { // All packages are by default recently used. lenient().when(mDexUseManager.getPackageLastUsedAtMs(any())).thenReturn(RECENT_TIME_MS); - List<? extends SecondaryDexInfo> secondaryDexInfo = createSecondaryDexInfo(); + List<DetailedSecondaryDexInfo> secondaryDexInfo = createSecondaryDexInfo(); lenient().doReturn(secondaryDexInfo).when(mDexUseManager).getSecondaryDexInfo(eq(PKG_NAME)); + lenient() + .doReturn(secondaryDexInfo) + .when(mDexUseManager) + .getFilteredDetailedSecondaryDexInfo(eq(PKG_NAME)); simulateStorageNotLow(); @@ -794,6 +799,56 @@ public class ArtManagerLocalTest { mArtManagerLocal.snapshotBootImageProfile(mSnapshot); } + @Test + public void testCleanup() throws Exception { + // It should keep all artifacts. + doReturn(createGetDexoptStatusResult("speed-profile", "bg-dexopt", "location")) + .when(mArtd) + .getDexoptStatus(eq("/data/app/foo/base.apk"), eq("arm64"), any()); + doReturn(createGetDexoptStatusResult("verify", "cmdline", "location")) + .when(mArtd) + .getDexoptStatus(eq("/data/user/0/foo/1.apk"), eq("arm64"), any()); + + // It should only keep VDEX files. + doReturn(createGetDexoptStatusResult("verify", "vdex", "location")) + .when(mArtd) + .getDexoptStatus(eq("/data/app/foo/split_0.apk"), eq("arm64"), any()); + doReturn(createGetDexoptStatusResult("verify", "vdex", "location")) + .when(mArtd) + .getDexoptStatus(eq("/data/app/foo/split_0.apk"), eq("arm"), any()); + + // It should not keep any artifacts. + doReturn(createGetDexoptStatusResult("run-from-apk", "unknown", "unknown")) + .when(mArtd) + .getDexoptStatus(eq("/data/app/foo/base.apk"), eq("arm"), any()); + + when(mSnapshot.getPackageStates()).thenReturn(Map.of(PKG_NAME, mPkgState)); + mArtManagerLocal.cleanup(mSnapshot); + + verify(mArtd).cleanup( + inAnyOrderDeepEquals(AidlUtils.buildProfilePathForPrimaryRef(PKG_NAME, "primary"), + AidlUtils.buildProfilePathForPrimaryCur( + 0 /* userId */, PKG_NAME, "primary"), + AidlUtils.buildProfilePathForPrimaryCur( + 1 /* userId */, PKG_NAME, "primary"), + AidlUtils.buildProfilePathForPrimaryRef(PKG_NAME, "split_0.split"), + AidlUtils.buildProfilePathForPrimaryCur( + 0 /* userId */, PKG_NAME, "split_0.split"), + AidlUtils.buildProfilePathForPrimaryCur( + 1 /* userId */, PKG_NAME, "split_0.split"), + AidlUtils.buildProfilePathForSecondaryRef("/data/user/0/foo/1.apk"), + AidlUtils.buildProfilePathForSecondaryCur("/data/user/0/foo/1.apk")), + inAnyOrderDeepEquals(AidlUtils.buildArtifactsPath("/data/app/foo/base.apk", "arm64", + mIsInReadonlyPartition), + AidlUtils.buildArtifactsPath( + "/data/user/0/foo/1.apk", "arm64", false /* isInDalvikCache */)), + inAnyOrderDeepEquals( + VdexPath.artifactsPath(AidlUtils.buildArtifactsPath( + "/data/app/foo/split_0.apk", "arm64", mIsInReadonlyPartition)), + VdexPath.artifactsPath(AidlUtils.buildArtifactsPath( + "/data/app/foo/split_0.apk", "arm", mIsInReadonlyPartition)))); + } + private AndroidPackage createPackage(boolean multiSplit) { AndroidPackage pkg = mock(AndroidPackage.class); @@ -882,8 +937,8 @@ public class ArtManagerLocalTest { return getDexoptStatusResult; } - private List<? extends SecondaryDexInfo> createSecondaryDexInfo() throws Exception { - var dexInfo = mock(SecondaryDexInfo.class); + private List<DetailedSecondaryDexInfo> createSecondaryDexInfo() throws Exception { + var dexInfo = mock(DetailedSecondaryDexInfo.class); lenient().when(dexInfo.dexPath()).thenReturn("/data/user/0/foo/1.apk"); lenient().when(dexInfo.abiNames()).thenReturn(Set.of("arm64-v8a")); lenient().when(dexInfo.classLoaderContext()).thenReturn("CLC"); diff --git a/libartservice/service/javatests/com/android/server/art/BackgroundDexoptJobTest.java b/libartservice/service/javatests/com/android/server/art/BackgroundDexoptJobTest.java index b6a0a81953..c008fd4f8d 100644 --- a/libartservice/service/javatests/com/android/server/art/BackgroundDexoptJobTest.java +++ b/libartservice/service/javatests/com/android/server/art/BackgroundDexoptJobTest.java @@ -121,6 +121,8 @@ public class BackgroundDexoptJobTest { Result result = Utils.getFuture(mBackgroundDexoptJob.start()); assertThat(result).isInstanceOf(CompletedResult.class); assertThat(((CompletedResult) result).dexoptResult()).isSameInstanceAs(mDexoptResult); + + verify(mArtManagerLocal).cleanup(same(mSnapshot)); } @Test diff --git a/libartservice/service/javatests/com/android/server/art/model/DexoptParamsTest.java b/libartservice/service/javatests/com/android/server/art/model/DexoptParamsTest.java index aeff58c62f..60986414fa 100644 --- a/libartservice/service/javatests/com/android/server/art/model/DexoptParamsTest.java +++ b/libartservice/service/javatests/com/android/server/art/model/DexoptParamsTest.java @@ -36,6 +36,11 @@ public class DexoptParamsTest { } @Test(expected = IllegalArgumentException.class) + public void testBuildReasonVdex() { + new DexoptParams.Builder("vdex").setCompilerFilter("speed").setPriorityClass(90).build(); + } + + @Test(expected = IllegalArgumentException.class) public void testBuildInvalidCompilerFilter() { new DexoptParams.Builder("install").setCompilerFilter("invalid").build(); } diff --git a/runtime/oat_file.cc b/runtime/oat_file.cc index de14835b23..60f1393995 100644 --- a/runtime/oat_file.cc +++ b/runtime/oat_file.cc @@ -1844,7 +1844,7 @@ class OatFileBackedByVdex final : public OatFileBase { InstructionSetFeatures::FromCppDefines(); SafeMap<std::string, std::string> store; store.Put(OatHeader::kCompilerFilter, CompilerFilter::NameOfFilter(CompilerFilter::kVerify)); - store.Put(OatHeader::kCompilationReasonKey, "vdex"); + store.Put(OatHeader::kCompilationReasonKey, kReasonVdex); store.Put(OatHeader::kConcurrentCopying, gUseReadBarrier ? OatHeader::kTrueValue : OatHeader::kFalseValue); if (context != nullptr) { diff --git a/runtime/oat_file.h b/runtime/oat_file.h index d52723a98e..57fedfd60d 100644 --- a/runtime/oat_file.h +++ b/runtime/oat_file.h @@ -60,6 +60,10 @@ class FakeOatFile; } // namespace collector } // namespace gc +// A special compilation reason to indicate that only the VDEX file is usable. Keep in sync with +// `ArtConstants::REASON_VDEX` in artd/binder/com/android/server/art/ArtConstants.aidl. +static constexpr const char* kReasonVdex = "vdex"; + // OatMethodOffsets are currently 5x32-bits=160-bits long, so if we can // save even one OatMethodOffsets struct, the more complicated encoding // using a bitmap pays for itself since few classes will have 160 |