Add logic to eagerly resolve const-string for startup methods
Added dex2oat option --resolve-startup-const-strings=<true|false>
If true, this option causes the compiler driver to resolve all
const-strings that are referenced from methods marked as "startup" in
the profile.
Bug: 116059983
Test: test-art-host
Change-Id: I61cf9e945c125671fc4ab4b50458a911318a837f
diff --git a/build/Android.gtest.mk b/build/Android.gtest.mk
index 4badc5a..e2a0a39 100644
--- a/build/Android.gtest.mk
+++ b/build/Android.gtest.mk
@@ -61,6 +61,7 @@
StaticLeafMethods \
Statics \
StaticsFromCode \
+ StringLiterals \
Transaction \
XandY
@@ -174,7 +175,7 @@
ART_GTEST_dex_cache_test_DEX_DEPS := Main Packages MethodTypes
ART_GTEST_dexanalyze_test_DEX_DEPS := MultiDex
ART_GTEST_dexlayout_test_DEX_DEPS := ManyMethods
-ART_GTEST_dex2oat_test_DEX_DEPS := $(ART_GTEST_dex2oat_environment_tests_DEX_DEPS) ManyMethods Statics VerifierDeps MainUncompressed EmptyUncompressed
+ART_GTEST_dex2oat_test_DEX_DEPS := $(ART_GTEST_dex2oat_environment_tests_DEX_DEPS) ManyMethods Statics VerifierDeps MainUncompressed EmptyUncompressed StringLiterals
ART_GTEST_dex2oat_image_test_DEX_DEPS := $(ART_GTEST_dex2oat_environment_tests_DEX_DEPS) Statics VerifierDeps
ART_GTEST_exception_test_DEX_DEPS := ExceptionHandle
ART_GTEST_hiddenapi_test_DEX_DEPS := HiddenApi
diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc
index c5416d5..89ac308 100644
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -708,9 +708,9 @@
}
}
-static void ResolveConstStrings(CompilerDriver* driver,
- const std::vector<const DexFile*>& dex_files,
- TimingLogger* timings) {
+void CompilerDriver::ResolveConstStrings(const std::vector<const DexFile*>& dex_files,
+ bool only_startup_strings,
+ TimingLogger* timings) {
ScopedObjectAccess soa(Thread::Current());
StackHandleScope<1> hs(soa.Self());
ClassLinker* const class_linker = Runtime::Current()->GetClassLinker();
@@ -721,12 +721,18 @@
TimingLogger::ScopedTiming t("Resolve const-string Strings", timings);
for (ClassAccessor accessor : dex_file->GetClasses()) {
- if (!driver->IsClassToCompile(accessor.GetDescriptor())) {
+ if (!IsClassToCompile(accessor.GetDescriptor())) {
// Compilation is skipped, do not resolve const-string in code of this class.
// FIXME: Make sure that inlining honors this. b/26687569
continue;
}
for (const ClassAccessor::Method& method : accessor.GetMethods()) {
+ if (only_startup_strings &&
+ profile_compilation_info_ != nullptr &&
+ !profile_compilation_info_->GetMethodHotness(method.GetReference()).IsStartup()) {
+ continue;
+ }
+
// Resolve const-strings in the code. Done to have deterministic allocation behavior. Right
// now this is single-threaded for simplicity.
// TODO: Collect the relevant string indices in parallel, then allocate them sequentially
@@ -897,8 +903,10 @@
if (GetCompilerOptions().IsForceDeterminism() && GetCompilerOptions().IsBootImage()) {
// Resolve strings from const-string. Do this now to have a deterministic image.
- ResolveConstStrings(this, dex_files, timings);
+ ResolveConstStrings(dex_files, /*only_startup_strings=*/ false, timings);
VLOG(compiler) << "Resolve const-strings: " << GetMemoryUsageString(false);
+ } else if (GetCompilerOptions().ResolveStartupConstStrings()) {
+ ResolveConstStrings(dex_files, /*only_startup_strings=*/ true, timings);
}
Verify(class_loader, dex_files, timings);
diff --git a/compiler/driver/compiler_driver.h b/compiler/driver/compiler_driver.h
index 343f67c..9a83e55 100644
--- a/compiler/driver/compiler_driver.h
+++ b/compiler/driver/compiler_driver.h
@@ -430,6 +430,12 @@
typedef AtomicDexRefMap<MethodReference, CompiledMethod*> MethodTable;
private:
+ // Resolve const string literals that are loaded from dex code. If only_startup_strings is
+ // specified, only methods that are marked startup in the profile are resolved.
+ void ResolveConstStrings(const std::vector<const DexFile*>& dex_files,
+ bool only_startup_strings,
+ /*inout*/ TimingLogger* timings);
+
// All method references that this compiler has compiled.
MethodTable compiled_methods_;
diff --git a/compiler/driver/compiler_options.cc b/compiler/driver/compiler_options.cc
index 3ab9afc..6b0e456 100644
--- a/compiler/driver/compiler_options.cc
+++ b/compiler/driver/compiler_options.cc
@@ -69,6 +69,7 @@
force_determinism_(false),
deduplicate_code_(true),
count_hotness_in_compiled_code_(false),
+ resolve_startup_const_strings_(false),
register_allocation_strategy_(RegisterAllocator::kRegisterAllocatorDefault),
passes_to_run_(nullptr) {
}
diff --git a/compiler/driver/compiler_options.h b/compiler/driver/compiler_options.h
index e9cbf74..4a6bbfa 100644
--- a/compiler/driver/compiler_options.h
+++ b/compiler/driver/compiler_options.h
@@ -313,6 +313,10 @@
return count_hotness_in_compiled_code_;
}
+ bool ResolveStartupConstStrings() const {
+ return resolve_startup_const_strings_;
+ }
+
private:
bool ParseDumpInitFailures(const std::string& option, std::string* error_msg);
void ParseDumpCfgPasses(const StringPiece& option, UsageFn Usage);
@@ -392,6 +396,10 @@
// won't be atomic for performance reasons, so we accept races, just like in interpreter.
bool count_hotness_in_compiled_code_;
+ // Whether we eagerly resolve all of the const strings that are loaded from startup methods in the
+ // profile.
+ bool resolve_startup_const_strings_;
+
RegisterAllocator::Strategy register_allocation_strategy_;
// If not null, specifies optimization passes which will be run instead of defaults.
diff --git a/compiler/driver/compiler_options_map-inl.h b/compiler/driver/compiler_options_map-inl.h
index d4a582f..5a84495 100644
--- a/compiler/driver/compiler_options_map-inl.h
+++ b/compiler/driver/compiler_options_map-inl.h
@@ -80,6 +80,7 @@
if (map.Exists(Base::CountHotnessInCompiledCode)) {
options->count_hotness_in_compiled_code_ = true;
}
+ map.AssignIfExists(Base::ResolveStartupConstStrings, &options->resolve_startup_const_strings_);
if (map.Exists(Base::DumpTimings)) {
options->dump_timings_ = true;
@@ -184,6 +185,11 @@
.template WithType<std::string>()
.IntoKey(Map::RegisterAllocationStrategy)
+ .Define("--resolve-startup-const-strings=_")
+ .template WithType<bool>()
+ .WithValueMap({{"false", false}, {"true", true}})
+ .IntoKey(Map::ResolveStartupConstStrings)
+
.Define("--verbose-methods=_")
.template WithType<ParseStringList<','>>()
.IntoKey(Map::VerboseMethods);
diff --git a/compiler/driver/compiler_options_map.def b/compiler/driver/compiler_options_map.def
index 238cd46..a593240 100644
--- a/compiler/driver/compiler_options_map.def
+++ b/compiler/driver/compiler_options_map.def
@@ -52,13 +52,14 @@
COMPILER_OPTIONS_KEY (double, TopKProfileThreshold)
COMPILER_OPTIONS_KEY (bool, AbortOnHardVerifierFailure)
COMPILER_OPTIONS_KEY (bool, AbortOnSoftVerifierFailure)
+COMPILER_OPTIONS_KEY (bool, ResolveStartupConstStrings, false)
COMPILER_OPTIONS_KEY (std::string, DumpInitFailures)
COMPILER_OPTIONS_KEY (std::string, DumpCFG)
COMPILER_OPTIONS_KEY (Unit, DumpCFGAppend)
// TODO: Add type parser.
COMPILER_OPTIONS_KEY (std::string, RegisterAllocationStrategy)
COMPILER_OPTIONS_KEY (ParseStringList<','>, VerboseMethods)
-COMPILER_OPTIONS_KEY (bool, DeduplicateCode, true)
+COMPILER_OPTIONS_KEY (bool, DeduplicateCode, true)
COMPILER_OPTIONS_KEY (Unit, CountHotnessInCompiledCode)
COMPILER_OPTIONS_KEY (Unit, DumpTimings)
COMPILER_OPTIONS_KEY (Unit, DumpPassTimings)
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index 71cdfd2..5c19a27 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -477,6 +477,9 @@
UsageError(" compiling the apk. If specified, the string will be embedded verbatim in");
UsageError(" the key value store of the oat file.");
UsageError("");
+ UsageError(" --resolve-startup-const-strings=true|false: If true, the compiler eagerly");
+ UsageError(" resolves strings referenced from const-string of startup methods.");
+ UsageError("");
UsageError(" Example: --compilation-reason=install");
UsageError("");
std::cerr << "See log for usage error information\n";
diff --git a/dex2oat/dex2oat_test.cc b/dex2oat/dex2oat_test.cc
index a1fed5f..91b231b 100644
--- a/dex2oat/dex2oat_test.cc
+++ b/dex2oat/dex2oat_test.cc
@@ -39,6 +39,7 @@
#include "dex/dex_file_loader.h"
#include "dex2oat_environment_test.h"
#include "dex2oat_return_codes.h"
+#include "intern_table-inl.h"
#include "oat.h"
#include "oat_file.h"
#include "profile/profile_compilation_info.h"
@@ -2082,6 +2083,74 @@
EXPECT_EQ(header.GetImageSection(ImageHeader::kSectionArtFields).Size(), 0u);
}
+TEST_F(Dex2oatTest, AppImageResolveStrings) {
+ using Hotness = ProfileCompilationInfo::MethodHotness;
+ // Create a profile with the startup method marked.
+ ScratchFile profile_file;
+ std::vector<uint16_t> methods;
+ {
+ std::unique_ptr<const DexFile> dex(OpenTestDexFile("StringLiterals"));
+ for (size_t method_idx = 0; method_idx < dex->NumMethodIds(); ++method_idx) {
+ if (std::string(dex->GetMethodName(dex->GetMethodId(method_idx))) == "startUpMethod") {
+ methods.push_back(method_idx);
+ }
+ }
+ ASSERT_GT(methods.size(), 0u);
+ // Here, we build the profile from the method lists.
+ ProfileCompilationInfo info;
+ info.AddMethodsForDex(Hotness::kFlagStartup, dex.get(), methods.begin(), methods.end());
+ // Save the profile since we want to use it with dex2oat to produce an oat file.
+ ASSERT_TRUE(info.Save(profile_file.GetFd()));
+ }
+ const std::string out_dir = GetScratchDir();
+ const std::string odex_location = out_dir + "/base.odex";
+ const std::string app_image_location = out_dir + "/base.art";
+ GenerateOdexForTest(GetTestDexFileName("StringLiterals"),
+ odex_location,
+ CompilerFilter::Filter::kSpeedProfile,
+ { "--app-image-file=" + app_image_location,
+ "--resolve-startup-const-strings=true",
+ "--profile-file=" + profile_file.GetFilename()},
+ /* expect_success= */ true,
+ /* use_fd= */ false,
+ [](const OatFile&) {});
+ // Open our generated oat file.
+ std::string error_msg;
+ std::unique_ptr<OatFile> odex_file(OatFile::Open(/* zip_fd= */ -1,
+ odex_location.c_str(),
+ odex_location.c_str(),
+ /* requested_base= */ nullptr,
+ /* executable= */ false,
+ /* low_4gb= */ false,
+ odex_location.c_str(),
+ /* reservation= */ nullptr,
+ &error_msg));
+ ASSERT_TRUE(odex_file != nullptr);
+ // Check the strings in the app image intern table only contain the "startup" strigs.
+ {
+ ScopedObjectAccess soa(Thread::Current());
+ std::unique_ptr<gc::space::ImageSpace> space =
+ gc::space::ImageSpace::CreateFromAppImage(app_image_location.c_str(),
+ odex_file.get(),
+ &error_msg);
+ ASSERT_TRUE(space != nullptr) << error_msg;
+ std::set<std::string> seen;
+ InternTable intern_table;
+ intern_table.AddImageStringsToTable(space.get(), [&](InternTable::UnorderedSet& interns)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
+ for (const GcRoot<mirror::String>& str : interns) {
+ seen.insert(str.Read()->ToModifiedUtf8());
+ }
+ });
+ EXPECT_TRUE(seen.find("Loading ") != seen.end());
+ EXPECT_TRUE(seen.find("Starting up") != seen.end());
+ EXPECT_TRUE(seen.find("abcd.apk") != seen.end());
+ EXPECT_TRUE(seen.find("Unexpected error") == seen.end());
+ EXPECT_TRUE(seen.find("Shutting down!") == seen.end());
+ }
+}
+
+
TEST_F(Dex2oatClassLoaderContextTest, StoredClassLoaderContext) {
std::vector<std::unique_ptr<const DexFile>> dex_files = OpenTestDexFiles("MultiDex");
const std::string out_dir = GetScratchDir();
diff --git a/runtime/intern_table-inl.h b/runtime/intern_table-inl.h
index d8e5da8..6e5c903 100644
--- a/runtime/intern_table-inl.h
+++ b/runtime/intern_table-inl.h
@@ -19,6 +19,9 @@
#include "intern_table.h"
+// Required for ToModifiedUtf8 below.
+#include "mirror/string-inl.h"
+
namespace art {
template <typename Visitor>
diff --git a/test/StringLiterals/StringLiterals.java b/test/StringLiterals/StringLiterals.java
new file mode 100644
index 0000000..8dee666
--- /dev/null
+++ b/test/StringLiterals/StringLiterals.java
@@ -0,0 +1,28 @@
+/*
+ * Copyright (C) 2018 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.
+ */
+
+class StringLiterals {
+ void startUpMethod() {
+ String resource = "abcd.apk";
+ System.out.println("Starting up");
+ System.out.println("Loading " + resource);
+ }
+
+ void otherMethod() {
+ System.out.println("Unexpected error");
+ System.out.println("Shutting down!");
+ }
+}