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

11 files changed, 141 insertions, 7 deletions

diff --git a/build/Android.gtest.mk b/build/Android.gtest.mk
index 4badc5a907..e2a0a391ab 100644
--- a/build/Android.gtest.mk
+++ b/build/Android.gtest.mk
@@ -61,6 +61,7 @@ GTEST_DEX_DIRECTORIES := \
   StaticLeafMethods \
   Statics \
   StaticsFromCode \
+  StringLiterals \
   Transaction \
   XandY
 
@@ -174,7 +175,7 @@ ART_GTEST_compiler_driver_test_DEX_DEPS := AbstractMethod StaticLeafMethods Prof
 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 c5416d5a3d..89ac308fed 100644
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -708,9 +708,9 @@ void CompilerDriver::Resolve(jobject class_loader,
   }
 }
 
-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 @@ static void ResolveConstStrings(CompilerDriver* driver,
     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 @@ void CompilerDriver::PreCompile(jobject class_loader,
 
   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 343f67c6d5..9a83e55c96 100644
--- a/compiler/driver/compiler_driver.h
+++ b/compiler/driver/compiler_driver.h
@@ -430,6 +430,12 @@ class CompilerDriver {
   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 3ab9afc5d6..6b0e45629b 100644
--- a/compiler/driver/compiler_options.cc
+++ b/compiler/driver/compiler_options.cc
@@ -69,6 +69,7 @@ CompilerOptions::CompilerOptions()
       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 e9cbf74428..4a6bbfaae6 100644
--- a/compiler/driver/compiler_options.h
+++ b/compiler/driver/compiler_options.h
@@ -313,6 +313,10 @@ class CompilerOptions final {
     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 @@ class CompilerOptions final {
   // 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 d4a582fb35..5a844959c4 100644
--- a/compiler/driver/compiler_options_map-inl.h
+++ b/compiler/driver/compiler_options_map-inl.h
@@ -80,6 +80,7 @@ inline bool ReadCompilerOptions(Base& map, CompilerOptions* options, std::string
   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 @@ inline void AddCompilerOptionsArgumentParserOptions(Builder& b) {
           .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 238cd465df..a593240365 100644
--- a/compiler/driver/compiler_options_map.def
+++ b/compiler/driver/compiler_options_map.def
@@ -52,13 +52,14 @@ COMPILER_OPTIONS_KEY (Unit,                        Baseline)
 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 71cdfd2c08..5c19a27bb9 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -477,6 +477,9 @@ NO_RETURN static void Usage(const char* fmt, ...) {
   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 a1fed5f6d9..91b231b419 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 @@ TEST_F(Dex2oatTest, AppImageNoProfile) {
   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 d8e5da8209..6e5c9039d3 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 0000000000..8dee666fd7
--- /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!");
+    }
+}