From d46824b955bc8e01346d225fb28ef4e0ab78e7da Mon Sep 17 00:00:00 2001 From: Jiakai Zhang Date: Tue, 6 Dec 2022 10:35:29 +0000 Subject: Implement shell command "dump-profiles". This change does not add any new API because the functionality is only used by the shell command. This change also changes the behavior of "snapshot-app-profile" and "snapshot-boot-image-profile" to pave the way to eventually take over the legacy PM shell commands. Bug: 261564086 Test: `adb shell pm art dump-profiles` with a package that has multiple splits. Test: `adb shell pm art snapshot-app-profile` with a package that has multiple splits. Ignore-AOSP-First: ART Services. Change-Id: I67435d0ba63a655e58fe1186bddea3b16e2fa23a --- artd/artd.cc | 36 +++++++--- artd/artd_test.cc | 78 +++++++++++++++++++++- artd/binder/com/android/server/art/IArtd.aidl | 5 ++ .../android/server/art/MergeProfileOptions.aidl | 4 ++ 4 files changed, 113 insertions(+), 10 deletions(-) (limited to 'artd') diff --git a/artd/artd.cc b/artd/artd.cc index 73208a67d1..2e55537dae 100644 --- a/artd/artd.cc +++ b/artd/artd.cc @@ -588,6 +588,9 @@ ndk::ScopedAStatus Artd::mergeProfiles(const std::vector& in_profil for (const std::string& dex_file : in_dexFiles) { OR_RETURN_FATAL(ValidateDexPath(dex_file)); } + if (in_options.forceMerge + in_options.dumpOnly + in_options.dumpClassesAndMethods > 1) { + return Fatal("Only one of 'forceMerge', 'dumpOnly', and 'dumpClassesAndMethods' can be set"); + } CmdlineBuilder args; FdLogger fd_logger; @@ -622,6 +625,11 @@ ndk::ScopedAStatus Artd::mergeProfiles(const std::vector& in_profil OR_RETURN_NON_FATAL(NewFile::Create(output_profile_path, in_outputProfile->fsPermission)); if (in_referenceProfile.has_value()) { + if (in_options.forceMerge || in_options.dumpOnly || in_options.dumpClassesAndMethods) { + return Fatal( + "Reference profile must not be set when 'forceMerge', 'dumpOnly', or " + "'dumpClassesAndMethods' is set"); + } std::string reference_profile_path = OR_RETURN_FATAL(BuildProfileOrDmPath(*in_referenceProfile)); if (in_referenceProfile->getTag() == ProfilePath::dexMetadataPath) { @@ -630,8 +638,12 @@ ndk::ScopedAStatus Artd::mergeProfiles(const std::vector& in_profil OR_RETURN_NON_FATAL(CopyFile(reference_profile_path, *output_profile_file)); } - // profman is ok with this being an empty file when in_referenceProfile isn't set. - args.Add("--reference-profile-file-fd=%d", output_profile_file->Fd()); + if (in_options.dumpOnly || in_options.dumpClassesAndMethods) { + args.Add("--dump-output-to-fd=%d", output_profile_file->Fd()); + } else { + // profman is ok with this being an empty file when in_referenceProfile isn't set. + args.Add("--reference-profile-file-fd=%d", output_profile_file->Fd()); + } fd_logger.Add(*output_profile_file); std::vector> dex_files; @@ -642,12 +654,16 @@ ndk::ScopedAStatus Artd::mergeProfiles(const std::vector& in_profil dex_files.push_back(std::move(dex_file)); } - args.AddIfNonEmpty("--min-new-classes-percent-change=%s", - props_->GetOrEmpty("dalvik.vm.bgdexopt.new-classes-percent")) - .AddIfNonEmpty("--min-new-methods-percent-change=%s", - props_->GetOrEmpty("dalvik.vm.bgdexopt.new-methods-percent")) - .AddIf(in_options.forceMerge, "--force-merge") - .AddIf(in_options.forBootImage, "--boot-image-merge"); + if (in_options.dumpOnly || in_options.dumpClassesAndMethods) { + args.Add(in_options.dumpOnly ? "--dump-only" : "--dump-classes-and-methods"); + } else { + args.AddIfNonEmpty("--min-new-classes-percent-change=%s", + props_->GetOrEmpty("dalvik.vm.bgdexopt.new-classes-percent")) + .AddIfNonEmpty("--min-new-methods-percent-change=%s", + props_->GetOrEmpty("dalvik.vm.bgdexopt.new-methods-percent")) + .AddIf(in_options.forceMerge, "--force-merge") + .AddIf(in_options.forBootImage, "--boot-image-merge"); + } LOG(INFO) << "Running profman: " << Join(args.Get(), /*separator=*/" ") << "\nOpened FDs: " << fd_logger; @@ -666,7 +682,9 @@ ndk::ScopedAStatus Artd::mergeProfiles(const std::vector& in_profil } ProfmanResult::ProcessingResult expected_result = - in_options.forceMerge ? ProfmanResult::kSuccess : ProfmanResult::kCompile; + (in_options.forceMerge || in_options.dumpOnly || in_options.dumpClassesAndMethods) ? + ProfmanResult::kSuccess : + ProfmanResult::kCompile; if (result.value() != expected_result) { return NonFatal("profman returned an unexpected code: {}"_format(result.value())); } diff --git a/artd/artd_test.cc b/artd/artd_test.cc index e28342b2da..8eab8add1f 100644 --- a/artd/artd_test.cc +++ b/artd/artd_test.cc @@ -1614,7 +1614,7 @@ TEST_F(ArtdTest, mergeProfilesProfilesDontExist) { EXPECT_THAT(output_profile.profilePath.tmpPath, IsEmpty()); } -TEST_F(ArtdTest, mergeProfilesWithOptions) { +TEST_F(ArtdTest, mergeProfilesWithOptionsForceMerge) { PrimaryCurProfilePath profile_0_path{ .userId = 0, .packageName = "com.android.foo", .profileName = "primary"}; std::string profile_0_file = OR_FATAL(BuildPrimaryCurProfilePath(profile_0_path)); @@ -1649,6 +1649,82 @@ TEST_F(ArtdTest, mergeProfilesWithOptions) { EXPECT_THAT(output_profile.profilePath.tmpPath, Not(IsEmpty())); } +TEST_F(ArtdTest, mergeProfilesWithOptionsDumpOnly) { + PrimaryCurProfilePath profile_0_path{ + .userId = 0, .packageName = "com.android.foo", .profileName = "primary"}; + std::string profile_0_file = OR_FATAL(BuildPrimaryCurProfilePath(profile_0_path)); + CreateFile(profile_0_file, "def"); + + OutputProfile output_profile{.profilePath = profile_path_->get(), + .fsPermission = FsPermission{.uid = -1, .gid = -1}}; + output_profile.profilePath.id = ""; + output_profile.profilePath.tmpPath = ""; + + CreateFile(dex_file_); + + EXPECT_CALL(*mock_exec_utils_, + DoExecAndReturnCode( + WhenSplitBy("--", + _, + AllOf(Contains("--dump-only"), + Not(Contains(Flag("--reference-profile-file-fd=", _))))), + _, + _)) + .WillOnce(DoAll(WithArg<0>(WriteToFdFlag("--dump-output-to-fd=", "dump")), + Return(ProfmanResult::kSuccess))); + + bool result; + EXPECT_TRUE(artd_ + ->mergeProfiles({profile_0_path}, + std::nullopt, + &output_profile, + {dex_file_}, + {.dumpOnly = true}, + &result) + .isOk()); + EXPECT_TRUE(result); + EXPECT_THAT(output_profile.profilePath.id, Not(IsEmpty())); + CheckContent(output_profile.profilePath.tmpPath, "dump"); +} + +TEST_F(ArtdTest, mergeProfilesWithOptionsDumpClassesAndMethods) { + PrimaryCurProfilePath profile_0_path{ + .userId = 0, .packageName = "com.android.foo", .profileName = "primary"}; + std::string profile_0_file = OR_FATAL(BuildPrimaryCurProfilePath(profile_0_path)); + CreateFile(profile_0_file, "def"); + + OutputProfile output_profile{.profilePath = profile_path_->get(), + .fsPermission = FsPermission{.uid = -1, .gid = -1}}; + output_profile.profilePath.id = ""; + output_profile.profilePath.tmpPath = ""; + + CreateFile(dex_file_); + + EXPECT_CALL(*mock_exec_utils_, + DoExecAndReturnCode( + WhenSplitBy("--", + _, + AllOf(Contains("--dump-classes-and-methods"), + Not(Contains(Flag("--reference-profile-file-fd=", _))))), + _, + _)) + .WillOnce(DoAll(WithArg<0>(WriteToFdFlag("--dump-output-to-fd=", "dump")), + Return(ProfmanResult::kSuccess))); + + bool result; + EXPECT_TRUE(artd_ + ->mergeProfiles({profile_0_path}, + std::nullopt, + &output_profile, + {dex_file_}, + {.dumpClassesAndMethods = true}, + &result) + .isOk()); + EXPECT_TRUE(result); + EXPECT_THAT(output_profile.profilePath.id, Not(IsEmpty())); + CheckContent(output_profile.profilePath.tmpPath, "dump"); +} + } // namespace } // namespace artd } // namespace art diff --git a/artd/binder/com/android/server/art/IArtd.aidl b/artd/binder/com/android/server/art/IArtd.aidl index 237f8a9f04..4e456574f7 100644 --- a/artd/binder/com/android/server/art/IArtd.aidl +++ b/artd/binder/com/android/server/art/IArtd.aidl @@ -88,6 +88,11 @@ interface IArtd { * writes the merge result to `outputProfile` and fills `outputProfile.profilePath.id` and * `outputProfile.profilePath.tmpPath` if a merge has been performed. * + * When `options.forceMerge`, `options.dumpOnly`, or `options.dumpClassesAndMethods` is set, + * `referenceProfile` must not be set. I.e., all inputs must be provided by `profiles`. This is + * because the merge will always happen, and hence no reference profile is needed to calculate + * the diff. + * * Throws fatal and non-fatal errors. */ boolean mergeProfiles(in List profiles, diff --git a/artd/binder/com/android/server/art/MergeProfileOptions.aidl b/artd/binder/com/android/server/art/MergeProfileOptions.aidl index fb7db80ba9..2d007f9406 100644 --- a/artd/binder/com/android/server/art/MergeProfileOptions.aidl +++ b/artd/binder/com/android/server/art/MergeProfileOptions.aidl @@ -32,4 +32,8 @@ parcelable MergeProfileOptions { boolean forceMerge; /** --boot-image-merge */ boolean forBootImage; + /** --dump-only */ + boolean dumpOnly; + /** --dump-classes-and-methods */ + boolean dumpClassesAndMethods; } -- cgit v1.2.3-59-g8ed1b