Snap for 7483611 from 6b12726ac223fef46b919714b9d35e4854322fa4 to mainline-adbd-release
Change-Id: I8deee347b9ebce237709baf148d64e985c3a9d96
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index 00dd274..9c9aeea 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -1805,14 +1805,14 @@
}
bool ShouldCompileDexFilesIndividually() const {
- // Compile individually if we are specifically asked to, or
+ // Compile individually if we are allowed to, and
// 1. not building an image, and
// 2. not verifying a vdex file, and
// 3. using multidex, and
// 4. not doing any AOT compilation.
// This means extract, no-vdex verify, and quicken, will use the individual compilation
// mode (to reduce RAM used by the compiler).
- return compile_individually_ ||
+ return compile_individually_ &&
(!IsImage() && !update_input_vdex_ &&
compiler_options_->dex_files_for_oat_file_.size() > 1 &&
!CompilerFilter::IsAotCompilationEnabled(compiler_options_->GetCompilerFilter()));
diff --git a/libartbase/base/flags.cc b/libartbase/base/flags.cc
index 4b7e780..aaa7661 100644
--- a/libartbase/base/flags.cc
+++ b/libartbase/base/flags.cc
@@ -84,10 +84,11 @@
}
template <typename Value>
-Flag<Value>::Flag(const std::string& name, Value default_value) :
+Flag<Value>::Flag(const std::string& name, Value default_value, FlagType type) :
FlagBase(GenerateCmdLineArgName(name),
GenerateSysPropName(name),
- GeneratePhenotypeName(name)),
+ GeneratePhenotypeName(name),
+ type),
initialized_{false},
default_{default_value} {
ALL_FLAGS.push_front(this);
@@ -100,7 +101,12 @@
template <typename Value>
void Flag<Value>::Reload() {
- // The cmdline flags are loaded by the parsed_options infra.
+ initialized_ = true;
+
+ // The cmdline flags are loaded by the parsed_options infra. No action needed here.
+ if (type_ == FlagType::kCmdlineOnly) {
+ return;
+ }
// Load system properties.
from_system_property_ = std::nullopt;
@@ -116,8 +122,6 @@
if (server_config != kUndefinedValue) {
ParseValue(server_config, &from_server_setting_);
}
-
- initialized_ = true;
}
template <typename Value>
@@ -131,8 +135,16 @@
template <typename Value>
void Flag<Value>::Dump(std::ostream& oss) const {
- std::pair<Value, std::string> valueLoc = GetValueLocation();
- oss << "value: " << std::get<0>(valueLoc) << " (from " << std::get<1>(valueLoc) << ")";
+ std::pair<Value, FlagOrigin> valueOrigin = GetValueAndOrigin();
+ std::string origin;
+ switch (std::get<1>(valueOrigin)) {
+ case FlagOrigin::kDefaultValue: origin = "default_value"; break;
+ case FlagOrigin::kCmdlineArg: origin = "cmdline_arg"; break;
+ case FlagOrigin::kSystemProperty: origin = "system_property"; break;
+ case FlagOrigin::kServerSetting: origin = "server_setting"; break;
+ }
+
+ oss << "value: " << std::get<0>(valueOrigin) << " (from " << origin << ")";
oss << "\n default: " << default_;
oss << "\n " << command_line_argument_name_ << ": ";
diff --git a/libartbase/base/flags.h b/libartbase/base/flags.h
index 973a5da0..e71bb4a 100644
--- a/libartbase/base/flags.h
+++ b/libartbase/base/flags.h
@@ -43,6 +43,15 @@
namespace art {
+// Enum representing the type of the ART flag.
+enum class FlagType {
+ // A flag that only looks at the cmdline argument to retrieve its value.
+ kCmdlineOnly,
+ // A flag that also looks at system properties and device config
+ // (phenotype properties) when retrieving its value.
+ kDeviceConfig,
+};
+
// FlagMetaBase handles automatically adding flags to the command line parser. It is parameterized
// by all supported flag types. In general, this should be treated as though it does not exist and
// FlagBase, which is already specialized to the types we support, should be used instead.
@@ -51,10 +60,12 @@
public:
FlagMetaBase(const std::string&& command_line_argument_name,
const std::string&& system_property_name,
- const std::string&& server_setting_name) :
+ const std::string&& server_setting_name,
+ FlagType type) :
command_line_argument_name_(command_line_argument_name),
system_property_name_(system_property_name),
- server_setting_name_(server_setting_name) {}
+ server_setting_name_(server_setting_name),
+ type_(type) {}
virtual ~FlagMetaBase() {}
template <typename Builder>
@@ -124,6 +135,7 @@
const std::string command_line_argument_name_;
const std::string system_property_name_;
const std::string server_setting_name_;
+ FlagType type_;
};
using FlagBase = FlagMetaBase<bool, int, std::string>;
@@ -133,13 +145,21 @@
class FlagsTests;
+// Describes the possible origins of a flag value.
+enum class FlagOrigin {
+ kDefaultValue,
+ kCmdlineArg,
+ kSystemProperty,
+ kServerSetting,
+};
+
// This class defines a flag with a value of a particular type.
template <typename Value>
class Flag : public FlagBase {
public:
// Create a new Flag. The name parameter is used to generate the names from the various parameter
// sources. See the documentation on the Flags struct for an example.
- explicit Flag(const std::string& name, Value default_value = {});
+ Flag(const std::string& name, Value default_value, FlagType type);
virtual ~Flag();
@@ -151,26 +171,41 @@
// 3) cmdline flag
// 4) default value
ALWAYS_INLINE Value GetValue() const {
- return std::get<0>(GetValueLocation());
+ return std::get<0>(GetValueAndOrigin());
}
ALWAYS_INLINE Value operator()() const {
return GetValue();
}
- // Returns the value and the location of that value for the given flag.
- ALWAYS_INLINE std::pair<Value, std::string> GetValueLocation() const {
+ // Return the value of the flag as optional.
+ //
+ // Returns the value of the flag if and only if the flag is set via
+ // a server side setting, system property or a cmdline arg.
+ // Otherwise it returns nullopt (meaning this never returns the default value).
+ //
+ // This is useful for properties that do not have a good default natural value
+ // (e.g. file path arguments).
+ ALWAYS_INLINE std::optional<Value> GetValueOptional() const {
+ std::pair<Value, FlagOrigin> result = GetValueAndOrigin();
+ return std::get<1>(result) == FlagOrigin::kDefaultValue
+ ? std::nullopt
+ : std::make_optional(std::get<0>(result));
+ }
+
+ // Returns the value and the origin of that value for the given flag.
+ ALWAYS_INLINE std::pair<Value, FlagOrigin> GetValueAndOrigin() const {
DCHECK(initialized_);
if (from_server_setting_.has_value()) {
- return std::pair{from_server_setting_.value(), server_setting_name_};
+ return std::pair{from_server_setting_.value(), FlagOrigin::kServerSetting};
}
if (from_system_property_.has_value()) {
- return std::pair{from_system_property_.value(), system_property_name_};
+ return std::pair{from_system_property_.value(), FlagOrigin::kSystemProperty};
}
if (from_command_line_.has_value()) {
- return std::pair{from_command_line_.value(), command_line_argument_name_};
+ return std::pair{from_command_line_.value(), FlagOrigin::kCmdlineArg};
}
- return std::pair{default_, "default_value"};
+ return std::pair{default_, FlagOrigin::kDefaultValue};
}
void Dump(std::ostream& oss) const override;
@@ -199,7 +234,7 @@
//
// Example:
//
-// Flag<int> WriteMetricsToLog{"my-feature-test.flag", 42};
+// Flag<int> WriteMetricsToLog{"my-feature-test.flag", 42, FlagType::kDeviceConfig};
//
// This creates a boolean flag that can be read through gFlags.WriteMetricsToLog(). The default
// value is false. Note that the default value can be left unspecified, in which the value of the
@@ -221,7 +256,13 @@
struct Flags {
// Flag used to test the infra.
// TODO: can be removed once we add real flags.
- Flag<int> MyFeatureTestFlag{"my-feature-test.flag", /*default_value=*/ 42};
+ Flag<int> MyFeatureTestFlag{"my-feature-test.flag", 42, FlagType::kDeviceConfig};
+
+ // Metric infra flags.
+ Flag<bool> WriteMetricsToStatsd{ "metrics.write-to-statsd", false, FlagType::kDeviceConfig};
+ Flag<bool> WriteMetricsToLogcat{ "metrics.write-to-logcat", false, FlagType::kCmdlineOnly};
+ Flag<int> MetricsReportingPeriod{"metrics.reporting-period", 0, FlagType::kCmdlineOnly};
+ Flag<std::string> WriteMetricsToFile{"metrics.write-to-file", "", FlagType::kCmdlineOnly};
};
// This is the actual instance of all the flags.
diff --git a/libartbase/base/flags_test.cc b/libartbase/base/flags_test.cc
index 72f0431..b41df46 100644
--- a/libartbase/base/flags_test.cc
+++ b/libartbase/base/flags_test.cc
@@ -30,7 +30,7 @@
class TestFlag {
public:
// Takes control of the tmp_file pointer.
- explicit TestFlag(ScratchFile* tmp_file) {
+ TestFlag(ScratchFile* tmp_file, FlagType flag_type) {
tmp_file_.reset(tmp_file);
std::string tmp_name = tmp_file_->GetFilename();
@@ -43,7 +43,7 @@
cmd_line_name_ = flag_name_;
std::replace(cmd_line_name_.begin(), cmd_line_name_.end(), '.', '-');
- flag_.reset(new Flag<int>(flag_name_, /*default_value=*/ 42));
+ flag_.reset(new Flag<int>(flag_name_, /*default_value=*/ 42, flag_type));
}
void AssertCmdlineValue(bool has_value, int expected) {
@@ -104,7 +104,7 @@
//
// So we do it in SetUpRuntimeOptions.
virtual void SetUpRuntimeOptions(RuntimeOptions* options) {
- test_flag_.reset(new TestFlag(new ScratchFile()));
+ test_flag_.reset(new TestFlag(new ScratchFile(), FlagType::kDeviceConfig));
CommonRuntimeTest::SetUpRuntimeOptions(options);
}
@@ -116,7 +116,11 @@
std::unique_ptr<TestFlag> test_flag_;
};
-class FlagsTestsWithCmdLine : public FlagsTests {
+class FlagsTestsWithCmdLineBase : public FlagsTests {
+ public:
+ explicit FlagsTestsWithCmdLineBase(FlagType type) : flag_type_(type) {
+ }
+
protected:
virtual void TearDown() {
android::base::SetProperty(test_flag_->SystemProperty(), "");
@@ -125,10 +129,24 @@
}
virtual void SetUpRuntimeOptions(RuntimeOptions* options) {
- test_flag_.reset(new TestFlag(new ScratchFile()));
+ test_flag_.reset(new TestFlag(new ScratchFile(), flag_type_));
std::string option = "-X" + test_flag_->CmdLineName() + ":1";
options->emplace_back(option.c_str(), nullptr);
}
+
+ FlagType flag_type_;
+};
+
+class FlagsTestsWithCmdLine : public FlagsTestsWithCmdLineBase {
+ public:
+ FlagsTestsWithCmdLine() : FlagsTestsWithCmdLineBase(FlagType::kDeviceConfig) {
+ }
+};
+
+class FlagsTestsCmdLineOnly : public FlagsTestsWithCmdLineBase {
+ public:
+ FlagsTestsCmdLineOnly() : FlagsTestsWithCmdLineBase(FlagType::kCmdlineOnly) {
+ }
};
// Validate that when no flag is set, the default is taken and none of the other
@@ -202,4 +220,28 @@
ASSERT_EQ(test_flag_->Value(), 1);
}
+// Validate that cmdline only flags don't read system properties.
+TEST_F(FlagsTestsCmdLineOnly, CmdlineOnlyFlags) {
+ if (!android::base::SetProperty(test_flag_->SystemProperty(), "2")) {
+ LOG(ERROR) << "Release does not support property setting, skipping test: "
+ << test_flag_->SystemProperty();
+ return;
+ }
+
+ if (android::base::SetProperty(test_flag_->ServerSetting(), "3")) {
+ LOG(ERROR) << "Release does not support property setting, skipping test: "
+ << test_flag_->ServerSetting();
+ return;
+ }
+
+ FlagBase::ReloadAllFlags("test");
+
+ test_flag_->AssertCmdlineValue(true, 1);
+ test_flag_->AssertSysPropValue(false, 2);
+ test_flag_->AssertServerSettingValue(false, 3);
+ test_flag_->AssertDefaultValue(42);
+
+ ASSERT_EQ(test_flag_->Value(), 1);
+}
+
} // namespace art
diff --git a/libnativeloader/library_namespaces.cpp b/libnativeloader/library_namespaces.cpp
index 2360819..8b87338 100644
--- a/libnativeloader/library_namespaces.cpp
+++ b/libnativeloader/library_namespaces.cpp
@@ -344,15 +344,17 @@
}
}
- auto apex_ns_name = FindApexNamespaceName(dex_path);
- if (apex_ns_name.ok()) {
- const auto& jni_libs = apex_jni_libraries(*apex_ns_name);
- if (jni_libs != "") {
- auto apex_ns = NativeLoaderNamespace::GetExportedNamespace(*apex_ns_name, is_bridged);
- if (apex_ns.ok()) {
- linked = app_ns->Link(&apex_ns.value(), jni_libs);
- if (!linked.ok()) {
- return linked.error();
+ for (const std::string& each_jar_path : android::base::Split(dex_path, ":")) {
+ auto apex_ns_name = FindApexNamespaceName(each_jar_path);
+ if (apex_ns_name.ok()) {
+ const auto& jni_libs = apex_jni_libraries(*apex_ns_name);
+ if (jni_libs != "") {
+ auto apex_ns = NativeLoaderNamespace::GetExportedNamespace(*apex_ns_name, is_bridged);
+ if (apex_ns.ok()) {
+ linked = app_ns->Link(&apex_ns.value(), jni_libs);
+ if (!linked.ok()) {
+ return linked.error();
+ }
}
}
}
diff --git a/libnativeloader/native_loader_test.cpp b/libnativeloader/native_loader_test.cpp
index 79e5070..c1dd4aa 100644
--- a/libnativeloader/native_loader_test.cpp
+++ b/libnativeloader/native_loader_test.cpp
@@ -188,6 +188,7 @@
bool expected_link_with_platform_ns = true;
bool expected_link_with_art_ns = true;
bool expected_link_with_i18n_ns = true;
+ bool expected_link_with_conscrypt_ns = false;
bool expected_link_with_sphal_ns = !vendor_public_libraries().empty();
bool expected_link_with_vndk_ns = false;
bool expected_link_with_vndk_product_ns = false;
@@ -196,6 +197,7 @@
std::string expected_shared_libs_to_platform_ns = default_public_libraries();
std::string expected_shared_libs_to_art_ns = apex_public_libraries().at("com_android_art");
std::string expected_shared_libs_to_i18n_ns = apex_public_libraries().at("com_android_i18n");
+ std::string expected_shared_libs_to_conscrypt_ns = apex_jni_libraries("com_android_conscrypt");
std::string expected_shared_libs_to_sphal_ns = vendor_public_libraries();
std::string expected_shared_libs_to_vndk_ns = vndksp_libraries_vendor();
std::string expected_shared_libs_to_vndk_product_ns = vndksp_libraries_product();
@@ -255,6 +257,11 @@
StrEq(expected_shared_libs_to_neuralnetworks_ns)))
.WillOnce(Return(true));
}
+ if (expected_link_with_conscrypt_ns) {
+ EXPECT_CALL(*mock, mock_link_namespaces(Eq(IsBridged()), _, NsEq("com_android_conscrypt"),
+ StrEq(expected_shared_libs_to_conscrypt_ns)))
+ .WillOnce(Return(true));
+ }
}
void RunTest() {
@@ -336,6 +343,17 @@
RunTest();
}
+TEST_P(NativeLoaderTest_Create, SystemServerWithApexJars) {
+ dex_path = "/system/framework/services.jar:/apex/com.android.conscrypt/javalib/service-foo.jar";
+ is_shared = true;
+
+ expected_namespace_name = "classloader-namespace-shared";
+ expected_namespace_flags |= ANDROID_NAMESPACE_TYPE_SHARED;
+ expected_link_with_conscrypt_ns = true;
+ SetExpectations();
+ RunTest();
+}
+
TEST_P(NativeLoaderTest_Create, UnbundledProductApp) {
dex_path = "/product/app/foo/foo.apk";
is_shared = false;
diff --git a/libnativeloader/native_loader_test.h b/libnativeloader/native_loader_test.h
index cab18da..09c56e5 100644
--- a/libnativeloader/native_loader_test.h
+++ b/libnativeloader/native_loader_test.h
@@ -80,6 +80,11 @@
NAMESPACE_ENTRY("com_android_i18n"),
NAMESPACE_ENTRY("com_android_neuralnetworks"),
NAMESPACE_ENTRY("com_android_art"),
+
+ // TODO(b/191644631) This can be removed when the test becomes more test-friendly.
+ // This is added so that the test can exercise the JNI lib related behavior.
+ NAMESPACE_ENTRY("com_android_conscrypt"),
+
NAMESPACE_ENTRY("default"),
NAMESPACE_ENTRY("sphal"),
NAMESPACE_ENTRY("system"),
diff --git a/runtime/metrics/reporter.cc b/runtime/metrics/reporter.cc
index 4cf1ba5..26b55b8 100644
--- a/runtime/metrics/reporter.cc
+++ b/runtime/metrics/reporter.cc
@@ -16,6 +16,7 @@
#include "reporter.h"
+#include "base/flags.h"
#include "runtime.h"
#include "runtime_options.h"
#include "statsd.h"
@@ -121,10 +122,7 @@
LOG_STREAM(DEBUG) << "Shutdown request received";
running = false;
- // Do one final metrics report, if enabled.
- if (config_.report_metrics_on_shutdown) {
- ReportMetrics();
- }
+ ReportMetrics();
},
[&](RequestMetricsReportMessage message) {
LOG_STREAM(DEBUG) << "Explicit report request received";
@@ -178,14 +176,12 @@
}
}
-ReportingConfig ReportingConfig::FromRuntimeArguments(const RuntimeArgumentMap& args) {
- using M = RuntimeArgumentMap;
+ReportingConfig ReportingConfig::FromFlags() {
return {
- .dump_to_logcat = args.Exists(M::WriteMetricsToLog),
- .dump_to_statsd = args.GetOrDefault(M::WriteMetricsToStatsd),
- .dump_to_file = args.GetOptional(M::WriteMetricsToFile),
- .report_metrics_on_shutdown = !args.Exists(M::DisableFinalMetricsReport),
- .periodic_report_seconds = args.GetOptional(M::MetricsReportingPeriod),
+ .dump_to_logcat = gFlags.WriteMetricsToLogcat(),
+ .dump_to_file = gFlags.WriteMetricsToFile.GetValueOptional(),
+ .dump_to_statsd = gFlags.WriteMetricsToStatsd(),
+ .periodic_report_seconds = gFlags.MetricsReportingPeriod.GetValueOptional(),
};
}
diff --git a/runtime/metrics/reporter.h b/runtime/metrics/reporter.h
index 3ad55b0..3e8056b 100644
--- a/runtime/metrics/reporter.h
+++ b/runtime/metrics/reporter.h
@@ -28,7 +28,7 @@
// Defines the set of options for how metrics reporting happens.
struct ReportingConfig {
- static ReportingConfig FromRuntimeArguments(const RuntimeArgumentMap& args);
+ static ReportingConfig FromFlags();
// Causes metrics to be written to the log, which makes them show up in logcat.
bool dump_to_logcat{false};
@@ -39,11 +39,6 @@
// If set, provides a file name to enable metrics logging to a file.
std::optional<std::string> dump_to_file;
- // Indicates whether to report the final state of metrics on shutdown.
- //
- // Note that reporting only happens if some output, such as logcat, is enabled.
- bool report_metrics_on_shutdown{true};
-
// If set, metrics will be reported every time this many seconds elapses.
std::optional<unsigned int> periodic_report_seconds;
};
diff --git a/runtime/parsed_options.cc b/runtime/parsed_options.cc
index 7699b25..c85c951 100644
--- a/runtime/parsed_options.cc
+++ b/runtime/parsed_options.cc
@@ -412,25 +412,6 @@
.IntoKey(M::CorePlatformApiPolicy)
.Define("-Xuse-stderr-logger")
.IntoKey(M::UseStderrLogger)
- .Define("-Xwrite-metrics-to-log")
- .WithHelp("Enables writing ART metrics to logcat")
- .IntoKey(M::WriteMetricsToLog)
- .Define("-Xwrite-metrics-to-statsd=_")
- .WithType<bool>()
- .WithValueMap({{"false", false}, {"true", true}})
- .WithHelp("Enables writing ART metrics to statsd")
- .IntoKey(M::WriteMetricsToStatsd)
- .Define("-Xwrite-metrics-to-file=_")
- .WithHelp("Enables writing ART metrics to the given file")
- .WithType<std::string>()
- .IntoKey(M::WriteMetricsToFile)
- .Define("-Xdisable-final-metrics-report")
- .WithHelp("Disables reporting metrics when ART shuts down")
- .IntoKey(M::DisableFinalMetricsReport)
- .Define("-Xmetrics-reporting-period=_")
- .WithHelp("The time in seconds between metrics reports")
- .WithType<unsigned int>()
- .IntoKey(M::MetricsReportingPeriod)
.Define("-Xonly-use-system-oat-files")
.IntoKey(M::OnlyUseTrustedOatFiles)
.Define("-Xverifier-logging-threshold=_")
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index 74a563e..ff504e3 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -1838,7 +1838,7 @@
// Class-roots are setup, we can now finish initializing the JniIdManager.
GetJniIdManager()->Init(self);
- InitMetrics(runtime_options);
+ InitMetrics();
// Runtime initialization is largely done now.
// We load plugins first since that can modify the runtime state slightly.
@@ -1938,8 +1938,8 @@
return true;
}
-void Runtime::InitMetrics(const RuntimeArgumentMap& runtime_options) {
- auto metrics_config = metrics::ReportingConfig::FromRuntimeArguments(runtime_options);
+void Runtime::InitMetrics() {
+ auto metrics_config = metrics::ReportingConfig::FromFlags();
metrics_reporter_ = metrics::MetricsReporter::Create(metrics_config, this);
}
diff --git a/runtime/runtime.h b/runtime/runtime.h
index 9026e3d..5bea321 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -1028,7 +1028,7 @@
SHARED_TRYLOCK_FUNCTION(true, Locks::mutator_lock_);
void InitNativeMethods() REQUIRES(!Locks::mutator_lock_);
void RegisterRuntimeNativeMethods(JNIEnv* env);
- void InitMetrics(const RuntimeArgumentMap& runtime_options);
+ void InitMetrics();
void StartDaemonThreads();
void StartSignalCatcher();
diff --git a/runtime/runtime_options.def b/runtime/runtime_options.def
index d1b37e5..b73f5c1 100644
--- a/runtime/runtime_options.def
+++ b/runtime/runtime_options.def
@@ -191,11 +191,4 @@
// This is to enable/disable Perfetto Java Heap Stack Profiling
RUNTIME_OPTIONS_KEY (bool, PerfettoJavaHeapStackProf, false)
-// Metrics configuration settings
-RUNTIME_OPTIONS_KEY (Unit, WriteMetricsToLog)
-RUNTIME_OPTIONS_KEY (bool, WriteMetricsToStatsd, true)
-RUNTIME_OPTIONS_KEY (std::string, WriteMetricsToFile)
-RUNTIME_OPTIONS_KEY (Unit, DisableFinalMetricsReport)
-RUNTIME_OPTIONS_KEY (unsigned int, MetricsReportingPeriod)
-
#undef RUNTIME_OPTIONS_KEY
diff --git a/test/2232-write-metrics-to-log/run b/test/2232-write-metrics-to-log/run
index b170317..07c1e60 100755
--- a/test/2232-write-metrics-to-log/run
+++ b/test/2232-write-metrics-to-log/run
@@ -15,4 +15,4 @@
# limitations under the License.
export ANDROID_LOG_TAGS="*:i"
-exec ${RUN} $@ --external-log-tags --runtime-option -Xwrite-metrics-to-log
+exec ${RUN} $@ --external-log-tags --runtime-option -Xmetrics-write-to-logcat:true
