Merge "ART: Change contention logging lock strategy"
diff --git a/cmdline/cmdline.h b/cmdline/cmdline.h
index 98010d7..18ca944 100644
--- a/cmdline/cmdline.h
+++ b/cmdline/cmdline.h
@@ -295,7 +295,7 @@
template <typename Args = CmdlineArgs>
struct CmdlineMain {
int Main(int argc, char** argv) {
- InitLogging(argv, Runtime::Aborter);
+ InitLogging(argv, Runtime::Abort);
std::unique_ptr<Args> args = std::unique_ptr<Args>(CreateArguments());
args_ = args.get();
diff --git a/cmdline/cmdline_parser_test.cc b/cmdline/cmdline_parser_test.cc
index b224ec7..d957869 100644
--- a/cmdline/cmdline_parser_test.cc
+++ b/cmdline/cmdline_parser_test.cc
@@ -123,7 +123,7 @@
using RuntimeParser = ParsedOptions::RuntimeParser;
static void SetUpTestCase() {
- art::InitLogging(nullptr, art::Runtime::Aborter); // argv = null
+ art::InitLogging(nullptr, art::Runtime::Abort); // argv = null
}
virtual void SetUp() {
diff --git a/dex2oat/dex2oat.cc b/dex2oat/dex2oat.cc
index c0ab21d..92d60b2 100644
--- a/dex2oat/dex2oat.cc
+++ b/dex2oat/dex2oat.cc
@@ -1133,7 +1133,7 @@
original_argc = argc;
original_argv = argv;
- InitLogging(argv, Runtime::Aborter);
+ InitLogging(argv, Runtime::Abort);
// Skip over argv[0].
argv++;
diff --git a/dexdump/dexdump_main.cc b/dexdump/dexdump_main.cc
index 74cae3c..606d4f8 100644
--- a/dexdump/dexdump_main.cc
+++ b/dexdump/dexdump_main.cc
@@ -60,7 +60,7 @@
*/
int dexdumpDriver(int argc, char** argv) {
// Art specific set up.
- InitLogging(argv, Runtime::Aborter);
+ InitLogging(argv, Runtime::Abort);
MemMap::Init();
// Reset options.
diff --git a/dexlayout/dexdiag.cc b/dexlayout/dexdiag.cc
index 78860e3..c14cd5f 100644
--- a/dexlayout/dexdiag.cc
+++ b/dexlayout/dexdiag.cc
@@ -465,7 +465,7 @@
}
// Art specific set up.
- InitLogging(argv, Runtime::Aborter);
+ InitLogging(argv, Runtime::Abort);
MemMap::Init();
#ifdef ART_TARGET_ANDROID
diff --git a/dexlayout/dexlayout_main.cc b/dexlayout/dexlayout_main.cc
index 3c627ea..33d62de 100644
--- a/dexlayout/dexlayout_main.cc
+++ b/dexlayout/dexlayout_main.cc
@@ -67,7 +67,7 @@
*/
int DexlayoutDriver(int argc, char** argv) {
// Art specific set up.
- InitLogging(argv, Runtime::Aborter);
+ InitLogging(argv, Runtime::Abort);
MemMap::Init();
Options options;
diff --git a/dexlist/dexlist.cc b/dexlist/dexlist.cc
index 29707af..fa232a7 100644
--- a/dexlist/dexlist.cc
+++ b/dexlist/dexlist.cc
@@ -211,7 +211,7 @@
*/
int dexlistDriver(int argc, char** argv) {
// Art specific set up.
- InitLogging(argv, Runtime::Aborter);
+ InitLogging(argv, Runtime::Abort);
MemMap::Init();
// Reset options.
diff --git a/dexoptanalyzer/dexoptanalyzer.cc b/dexoptanalyzer/dexoptanalyzer.cc
index 9a2eb7f..e2c159a 100644
--- a/dexoptanalyzer/dexoptanalyzer.cc
+++ b/dexoptanalyzer/dexoptanalyzer.cc
@@ -127,7 +127,7 @@
original_argc = argc;
original_argv = argv;
- InitLogging(argv, Runtime::Aborter);
+ InitLogging(argv, Runtime::Abort);
// Skip over the command name.
argv++;
argc--;
diff --git a/patchoat/patchoat.cc b/patchoat/patchoat.cc
index 848eb8d..149960e 100644
--- a/patchoat/patchoat.cc
+++ b/patchoat/patchoat.cc
@@ -783,7 +783,7 @@
}
static int patchoat(int argc, char **argv) {
- InitLogging(argv, Runtime::Aborter);
+ InitLogging(argv, Runtime::Abort);
MemMap::Init();
const bool debug = kIsDebugBuild;
orig_argc = argc;
diff --git a/profman/profman.cc b/profman/profman.cc
index 14b0262..94e81c7 100644
--- a/profman/profman.cc
+++ b/profman/profman.cc
@@ -189,7 +189,7 @@
original_argc = argc;
original_argv = argv;
- InitLogging(argv, Runtime::Aborter);
+ InitLogging(argv, Runtime::Abort);
// Skip over the command name.
argv++;
diff --git a/runtime/art_method.h b/runtime/art_method.h
index d537764..396c878 100644
--- a/runtime/art_method.h
+++ b/runtime/art_method.h
@@ -451,7 +451,11 @@
}
ProfilingInfo* GetProfilingInfo(PointerSize pointer_size) {
- DCHECK(!IsNative());
+ // Don't do a read barrier in the DCHECK, as GetProfilingInfo is called in places
+ // where the declaring class is treated as a weak reference (accessing it with
+ // a read barrier would either prevent unloading the class, or crash the runtime if
+ // the GC wants to unload it).
+ DCHECK(!IsNative<kWithoutReadBarrier>());
return reinterpret_cast<ProfilingInfo*>(GetDataPtrSize(pointer_size));
}
diff --git a/runtime/base/logging.cc b/runtime/base/logging.cc
index 2be9067..4776357 100644
--- a/runtime/base/logging.cc
+++ b/runtime/base/logging.cc
@@ -85,7 +85,7 @@
LogVerbosity gLogVerbosity;
-unsigned int gAborting = 0;
+std::atomic<unsigned int> gAborting(0);
static std::unique_ptr<std::string> gCmdLine;
static std::unique_ptr<std::string> gProgramInvocationName;
diff --git a/runtime/base/logging.h b/runtime/base/logging.h
index d8954e5..15f9353 100644
--- a/runtime/base/logging.h
+++ b/runtime/base/logging.h
@@ -102,7 +102,7 @@
// 0 if not abort, non-zero if an abort is in progress. Used on fatal exit to prevents recursive
// aborts. Global declaration allows us to disable some error checking to ensure fatal shutdown
// makes forward progress.
-extern unsigned int gAborting;
+extern std::atomic<unsigned int> gAborting;
// Configure logging based on ANDROID_LOG_TAGS environment variable.
// We need to parse a string that looks like
diff --git a/runtime/base/variant_map.h b/runtime/base/variant_map.h
index fdb60c4..d87df87 100644
--- a/runtime/base/variant_map.h
+++ b/runtime/base/variant_map.h
@@ -22,6 +22,7 @@
#include <type_traits>
#include <utility>
+#include "android-base/logging.h"
#include "base/stl_util_identity.h"
namespace art {
@@ -278,7 +279,8 @@
auto* new_value = new TValue(value);
Remove(key);
- storage_map_.insert({{key.Clone(), new_value}});
+ bool inserted = storage_map_.insert({key.Clone(), new_value}).second;
+ DCHECK(inserted); // ensure key.Clone() does not leak memory.
}
// Set a value for a given key, only if there was no previous value before.
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index 71558e1..141df1e 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -5440,184 +5440,6 @@
return true;
}
-static void CountMethodsAndFields(ClassDataItemIterator& dex_data,
- size_t* virtual_methods,
- size_t* direct_methods,
- size_t* static_fields,
- size_t* instance_fields) {
- *static_fields = dex_data.NumStaticFields();
- *instance_fields = dex_data.NumInstanceFields();
- *direct_methods = dex_data.NumDirectMethods();
- *virtual_methods = dex_data.NumVirtualMethods();
-}
-
-static void DumpClass(std::ostream& os,
- const DexFile& dex_file, const DexFile::ClassDef& dex_class_def,
- const char* suffix) {
- ClassDataItemIterator dex_data(dex_file, dex_file.GetClassData(dex_class_def));
- os << dex_file.GetClassDescriptor(dex_class_def) << suffix << ":\n";
- os << " Static fields:\n";
- while (dex_data.HasNextStaticField()) {
- const DexFile::FieldId& id = dex_file.GetFieldId(dex_data.GetMemberIndex());
- os << " " << dex_file.GetFieldTypeDescriptor(id) << " " << dex_file.GetFieldName(id) << "\n";
- dex_data.Next();
- }
- os << " Instance fields:\n";
- while (dex_data.HasNextInstanceField()) {
- const DexFile::FieldId& id = dex_file.GetFieldId(dex_data.GetMemberIndex());
- os << " " << dex_file.GetFieldTypeDescriptor(id) << " " << dex_file.GetFieldName(id) << "\n";
- dex_data.Next();
- }
- os << " Direct methods:\n";
- while (dex_data.HasNextDirectMethod()) {
- const DexFile::MethodId& id = dex_file.GetMethodId(dex_data.GetMemberIndex());
- os << " " << dex_file.GetMethodName(id) << dex_file.GetMethodSignature(id).ToString() << "\n";
- dex_data.Next();
- }
- os << " Virtual methods:\n";
- while (dex_data.HasNextVirtualMethod()) {
- const DexFile::MethodId& id = dex_file.GetMethodId(dex_data.GetMemberIndex());
- os << " " << dex_file.GetMethodName(id) << dex_file.GetMethodSignature(id).ToString() << "\n";
- dex_data.Next();
- }
-}
-
-static std::string DumpClasses(const DexFile& dex_file1,
- const DexFile::ClassDef& dex_class_def1,
- const DexFile& dex_file2,
- const DexFile::ClassDef& dex_class_def2) {
- std::ostringstream os;
- DumpClass(os, dex_file1, dex_class_def1, " (Compile time)");
- DumpClass(os, dex_file2, dex_class_def2, " (Runtime)");
- return os.str();
-}
-
-
-// Very simple structural check on whether the classes match. Only compares the number of
-// methods and fields.
-static bool SimpleStructuralCheck(const DexFile& dex_file1,
- const DexFile::ClassDef& dex_class_def1,
- const DexFile& dex_file2,
- const DexFile::ClassDef& dex_class_def2,
- std::string* error_msg) {
- ClassDataItemIterator dex_data1(dex_file1, dex_file1.GetClassData(dex_class_def1));
- ClassDataItemIterator dex_data2(dex_file2, dex_file2.GetClassData(dex_class_def2));
-
- // Counters for current dex file.
- size_t dex_virtual_methods1, dex_direct_methods1, dex_static_fields1, dex_instance_fields1;
- CountMethodsAndFields(dex_data1,
- &dex_virtual_methods1,
- &dex_direct_methods1,
- &dex_static_fields1,
- &dex_instance_fields1);
- // Counters for compile-time dex file.
- size_t dex_virtual_methods2, dex_direct_methods2, dex_static_fields2, dex_instance_fields2;
- CountMethodsAndFields(dex_data2,
- &dex_virtual_methods2,
- &dex_direct_methods2,
- &dex_static_fields2,
- &dex_instance_fields2);
-
- if (dex_virtual_methods1 != dex_virtual_methods2) {
- std::string class_dump = DumpClasses(dex_file1, dex_class_def1, dex_file2, dex_class_def2);
- *error_msg = StringPrintf("Virtual method count off: %zu vs %zu\n%s",
- dex_virtual_methods1,
- dex_virtual_methods2,
- class_dump.c_str());
- return false;
- }
- if (dex_direct_methods1 != dex_direct_methods2) {
- std::string class_dump = DumpClasses(dex_file1, dex_class_def1, dex_file2, dex_class_def2);
- *error_msg = StringPrintf("Direct method count off: %zu vs %zu\n%s",
- dex_direct_methods1,
- dex_direct_methods2,
- class_dump.c_str());
- return false;
- }
- if (dex_static_fields1 != dex_static_fields2) {
- std::string class_dump = DumpClasses(dex_file1, dex_class_def1, dex_file2, dex_class_def2);
- *error_msg = StringPrintf("Static field count off: %zu vs %zu\n%s",
- dex_static_fields1,
- dex_static_fields2,
- class_dump.c_str());
- return false;
- }
- if (dex_instance_fields1 != dex_instance_fields2) {
- std::string class_dump = DumpClasses(dex_file1, dex_class_def1, dex_file2, dex_class_def2);
- *error_msg = StringPrintf("Instance field count off: %zu vs %zu\n%s",
- dex_instance_fields1,
- dex_instance_fields2,
- class_dump.c_str());
- return false;
- }
-
- return true;
-}
-
-// Checks whether a the super-class changed from what we had at compile-time. This would
-// invalidate quickening.
-static bool CheckSuperClassChange(Handle<mirror::Class> klass,
- const DexFile& dex_file,
- const DexFile::ClassDef& class_def,
- ObjPtr<mirror::Class> super_class)
- REQUIRES_SHARED(Locks::mutator_lock_) {
- // Check for unexpected changes in the superclass.
- // Quick check 1) is the super_class class-loader the boot class loader? This always has
- // precedence.
- if (super_class->GetClassLoader() != nullptr &&
- // Quick check 2) different dex cache? Breaks can only occur for different dex files,
- // which is implied by different dex cache.
- klass->GetDexCache() != super_class->GetDexCache()) {
- // Now comes the expensive part: things can be broken if (a) the klass' dex file has a
- // definition for the super-class, and (b) the files are in separate oat files. The oat files
- // are referenced from the dex file, so do (b) first. Only relevant if we have oat files.
- const OatDexFile* class_oat_dex_file = dex_file.GetOatDexFile();
- const OatFile* class_oat_file = nullptr;
- if (class_oat_dex_file != nullptr) {
- class_oat_file = class_oat_dex_file->GetOatFile();
- }
-
- if (class_oat_file != nullptr) {
- const OatDexFile* loaded_super_oat_dex_file = super_class->GetDexFile().GetOatDexFile();
- const OatFile* loaded_super_oat_file = nullptr;
- if (loaded_super_oat_dex_file != nullptr) {
- loaded_super_oat_file = loaded_super_oat_dex_file->GetOatFile();
- }
-
- if (loaded_super_oat_file != nullptr && class_oat_file != loaded_super_oat_file) {
- // Now check (a).
- const DexFile::ClassDef* super_class_def = dex_file.FindClassDef(class_def.superclass_idx_);
- if (super_class_def != nullptr) {
- // Uh-oh, we found something. Do our check.
- std::string error_msg;
- if (!SimpleStructuralCheck(dex_file, *super_class_def,
- super_class->GetDexFile(), *super_class->GetClassDef(),
- &error_msg)) {
- // Print a warning to the log. This exception might be caught, e.g., as common in test
- // drivers. When the class is later tried to be used, we re-throw a new instance, as we
- // only save the type of the exception.
- LOG(WARNING) << "Incompatible structural change detected: " <<
- StringPrintf(
- "Structural change of %s is hazardous (%s at compile time, %s at runtime): %s",
- dex_file.PrettyType(super_class_def->class_idx_).c_str(),
- class_oat_file->GetLocation().c_str(),
- loaded_super_oat_file->GetLocation().c_str(),
- error_msg.c_str());
- ThrowIncompatibleClassChangeError(klass.Get(),
- "Structural change of %s is hazardous (%s at compile time, %s at runtime): %s",
- dex_file.PrettyType(super_class_def->class_idx_).c_str(),
- class_oat_file->GetLocation().c_str(),
- loaded_super_oat_file->GetLocation().c_str(),
- error_msg.c_str());
- return false;
- }
- }
- }
- }
- }
- return true;
-}
-
bool ClassLinker::LoadSuperAndInterfaces(Handle<mirror::Class> klass, const DexFile& dex_file) {
CHECK_EQ(mirror::Class::kStatusIdx, klass->GetStatus());
const DexFile::ClassDef& class_def = dex_file.GetClassDef(klass->GetDexClassDefIndex());
@@ -5650,11 +5472,6 @@
}
CHECK(super_class->IsResolved());
klass->SetSuperClass(super_class);
-
- if (!CheckSuperClassChange(klass, dex_file, class_def, super_class)) {
- DCHECK(Thread::Current()->IsExceptionPending());
- return false;
- }
}
const DexFile::TypeList* interfaces = dex_file.GetInterfacesList(class_def);
if (interfaces != nullptr) {
diff --git a/runtime/common_runtime_test.cc b/runtime/common_runtime_test.cc
index 5beac1a..6441a44 100644
--- a/runtime/common_runtime_test.cc
+++ b/runtime/common_runtime_test.cc
@@ -59,7 +59,7 @@
// everything else. In case you want to see all messages, comment out the line.
setenv("ANDROID_LOG_TAGS", "*:e", 1);
- art::InitLogging(argv, art::Runtime::Aborter);
+ art::InitLogging(argv, art::Runtime::Abort);
LOG(INFO) << "Running main() from common_runtime_test.cc...";
testing::InitGoogleTest(&argc, argv);
return RUN_ALL_TESTS();
diff --git a/runtime/openjdkjvmti/ti_stack.cc b/runtime/openjdkjvmti/ti_stack.cc
index a17226c..edb6ffe 100644
--- a/runtime/openjdkjvmti/ti_stack.cc
+++ b/runtime/openjdkjvmti/ti_stack.cc
@@ -39,6 +39,7 @@
#include "art_field-inl.h"
#include "art_method-inl.h"
#include "art_jvmti.h"
+#include "barrier.h"
#include "base/bit_utils.h"
#include "base/enums.h"
#include "base/mutex.h"
@@ -299,13 +300,20 @@
template <typename Data>
struct GetAllStackTracesVectorClosure : public art::Closure {
- GetAllStackTracesVectorClosure(size_t stop, Data* data_) : stop_input(stop), data(data_) {}
+ GetAllStackTracesVectorClosure(size_t stop, Data* data_)
+ : barrier(0), stop_input(stop), data(data_) {}
void Run(art::Thread* thread) OVERRIDE
REQUIRES_SHARED(art::Locks::mutator_lock_)
REQUIRES(!data->mutex) {
art::Thread* self = art::Thread::Current();
+ Work(thread, self);
+ barrier.Pass(self);
+ }
+ void Work(art::Thread* thread, art::Thread* self)
+ REQUIRES_SHARED(art::Locks::mutator_lock_)
+ REQUIRES(!data->mutex) {
// Skip threads that are still starting.
if (thread->IsStillStarting()) {
return;
@@ -324,10 +332,23 @@
visitor.WalkStack(/* include_transitions */ false);
}
+ art::Barrier barrier;
const size_t stop_input;
Data* data;
};
+template <typename Data>
+static void RunCheckpointAndWait(Data* data, size_t max_frame_count) {
+ GetAllStackTracesVectorClosure<Data> closure(max_frame_count, data);
+ size_t barrier_count = art::Runtime::Current()->GetThreadList()->RunCheckpoint(&closure, nullptr);
+ if (barrier_count == 0) {
+ return;
+ }
+ art::Thread* self = art::Thread::Current();
+ art::ScopedThreadStateChange tsc(self, art::ThreadState::kWaitingForCheckPointsToRun);
+ closure.barrier.Increment(self, barrier_count);
+}
+
jvmtiError StackUtil::GetAllStackTraces(jvmtiEnv* env,
jint max_frame_count,
jvmtiStackInfo** stack_info_ptr,
@@ -375,9 +396,7 @@
};
AllStackTracesData data;
- GetAllStackTracesVectorClosure<AllStackTracesData> closure(
- static_cast<size_t>(max_frame_count), &data);
- art::Runtime::Current()->GetThreadList()->RunCheckpoint(&closure, nullptr);
+ RunCheckpointAndWait(&data, static_cast<size_t>(max_frame_count));
art::Thread* current = art::Thread::Current();
@@ -538,9 +557,7 @@
data.handles.push_back(hs.NewHandle(soa.Decode<art::mirror::Object>(thread_list[i])));
}
- GetAllStackTracesVectorClosure<SelectStackTracesData> closure(
- static_cast<size_t>(max_frame_count), &data);
- art::Runtime::Current()->GetThreadList()->RunCheckpoint(&closure, nullptr);
+ RunCheckpointAndWait(&data, static_cast<size_t>(max_frame_count));
// Convert the data into our output format.
diff --git a/runtime/runtime.cc b/runtime/runtime.cc
index c11e4bd..46b4392 100644
--- a/runtime/runtime.cc
+++ b/runtime/runtime.cc
@@ -483,7 +483,16 @@
};
void Runtime::Abort(const char* msg) {
- gAborting++; // set before taking any locks
+ auto old_value = gAborting.fetch_add(1); // set before taking any locks
+
+#ifdef ART_TARGET_ANDROID
+ if (old_value == 0) {
+ // Only set the first abort message.
+ android_set_abort_message(msg);
+ }
+#else
+ UNUSED(old_value);
+#endif
// Ensure that we don't have multiple threads trying to abort at once,
// which would result in significantly worse diagnostics.
@@ -570,7 +579,7 @@
bool Runtime::ParseOptions(const RuntimeOptions& raw_options,
bool ignore_unrecognized,
RuntimeArgumentMap* runtime_options) {
- InitLogging(/* argv */ nullptr, Aborter); // Calls Locks::Init() as a side effect.
+ InitLogging(/* argv */ nullptr, Abort); // Calls Locks::Init() as a side effect.
bool parsed = ParsedOptions::Parse(raw_options, ignore_unrecognized, runtime_options);
if (!parsed) {
LOG(ERROR) << "Failed to parse options";
@@ -2358,14 +2367,6 @@
}
}
-NO_RETURN
-void Runtime::Aborter(const char* abort_message) {
-#ifdef ART_TARGET_ANDROID
- android_set_abort_message(abort_message);
-#endif
- Runtime::Abort(abort_message);
-}
-
RuntimeCallbacks* Runtime::GetRuntimeCallbacks() {
return callbacks_.get();
}
diff --git a/runtime/runtime.h b/runtime/runtime.h
index 2505d87..af9d215 100644
--- a/runtime/runtime.h
+++ b/runtime/runtime.h
@@ -649,9 +649,6 @@
return cha_;
}
- NO_RETURN
- static void Aborter(const char* abort_message);
-
void AttachAgent(const std::string& agent_arg);
const std::list<ti::Agent>& GetAgents() const {
diff --git a/test/131-structural-change/build b/test/131-structural-change/build
deleted file mode 100755
index ff0da20..0000000
--- a/test/131-structural-change/build
+++ /dev/null
@@ -1,39 +0,0 @@
-#!/bin/bash
-#
-# Copyright (C) 2015 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.
-
-# Stop if something fails.
-set -e
-
-if [ ${USE_JACK} = "true" ]; then
- ${JACK} --output-dex . src
- zip $TEST_NAME.jar classes.dex
-
- ${JACK} --output-dex . src-ex
- zip ${TEST_NAME}-ex.jar classes.dex
-else
- mkdir classes
- ${JAVAC} -d classes `find src -name '*.java'`
-
- mkdir classes-ex
- ${JAVAC} -d classes-ex `find src-ex -name '*.java'`
-
- if [ ${NEED_DEX} = "true" ]; then
- ${DX} -JXmx256m --debug --dex --dump-to=classes.lst --output=classes.dex --dump-width=1000 classes
- zip $TEST_NAME.jar classes.dex
- ${DX} -JXmx256m --debug --dex --dump-to=classes-ex.lst --output=classes.dex --dump-width=1000 classes-ex
- zip ${TEST_NAME}-ex.jar classes.dex
- fi
-fi
diff --git a/test/131-structural-change/expected.txt b/test/131-structural-change/expected.txt
deleted file mode 100644
index 1d19278..0000000
--- a/test/131-structural-change/expected.txt
+++ /dev/null
@@ -1,3 +0,0 @@
-JNI_OnLoad called
-Should really reach here.
-Done.
diff --git a/test/131-structural-change/info.txt b/test/131-structural-change/info.txt
deleted file mode 100644
index 6d5817b..0000000
--- a/test/131-structural-change/info.txt
+++ /dev/null
@@ -1 +0,0 @@
-Check whether a structural change in a (non-native) multi-dex scenario is detected.
diff --git a/test/131-structural-change/run b/test/131-structural-change/run
deleted file mode 100755
index 63fdb8c..0000000
--- a/test/131-structural-change/run
+++ /dev/null
@@ -1,18 +0,0 @@
-#!/bin/bash
-#
-# Copyright (C) 2015 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.
-
-# Use secondary switch to add secondary dex file to class path.
-exec ${RUN} "${@}" --secondary
diff --git a/test/131-structural-change/src-ex/A.java b/test/131-structural-change/src-ex/A.java
deleted file mode 100644
index 800347b..0000000
--- a/test/131-structural-change/src-ex/A.java
+++ /dev/null
@@ -1,20 +0,0 @@
-/*
- * Copyright (C) 2015 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.
- */
-
-public class A {
- public void bar() {
- }
-}
diff --git a/test/131-structural-change/src-ex/B.java b/test/131-structural-change/src-ex/B.java
deleted file mode 100644
index 61369db..0000000
--- a/test/131-structural-change/src-ex/B.java
+++ /dev/null
@@ -1,21 +0,0 @@
-/*
- * Copyright (C) 2015 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.
- */
-
-public class B extends A {
- public void test() {
- System.out.println("Should not reach this!");
- }
-}
diff --git a/test/131-structural-change/src/A.java b/test/131-structural-change/src/A.java
deleted file mode 100644
index b07de58..0000000
--- a/test/131-structural-change/src/A.java
+++ /dev/null
@@ -1,26 +0,0 @@
-/*
- * Copyright (C) 2015 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.
- */
-
-public class A {
- public void foo() {
- }
-
- public void bar() {
- }
-
- public void baz() {
- }
-}
diff --git a/test/131-structural-change/src/Main.java b/test/131-structural-change/src/Main.java
deleted file mode 100644
index c748899..0000000
--- a/test/131-structural-change/src/Main.java
+++ /dev/null
@@ -1,54 +0,0 @@
-/*
- * Copyright (C) 2015 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.
- */
-
-import java.io.File;
-import java.lang.reflect.Constructor;
-import java.lang.reflect.Method;
-
-/**
- * Structural hazard test.
- */
-public class Main {
- public static void main(String[] args) {
- System.loadLibrary(args[0]);
- new Main().run();
- }
-
- private void run() {
- try {
- Class<?> bClass = getClass().getClassLoader().loadClass("A");
- System.out.println("Should really reach here.");
- } catch (Exception e) {
- e.printStackTrace(System.out);
- }
-
- boolean haveOatFile = hasOatFile();
- boolean gotError = false;
- try {
- Class<?> bClass = getClass().getClassLoader().loadClass("B");
- } catch (IncompatibleClassChangeError icce) {
- gotError = true;
- } catch (Exception e) {
- e.printStackTrace(System.out);
- }
- if (haveOatFile ^ gotError) {
- System.out.println("Did not get expected error. " + haveOatFile + " " + gotError);
- }
- System.out.println("Done.");
- }
-
- private native static boolean hasOatFile();
-}
diff --git a/test/knownfailures.json b/test/knownfailures.json
index 8bdaeaa..7bfa417 100644
--- a/test/knownfailures.json
+++ b/test/knownfailures.json
@@ -143,15 +143,6 @@
"complete test) JDWP must be set up."]
},
{
- "tests": "131-structural-change",
- "variant": "debug",
- "description": ["131 is an old test. The functionality has been",
- "implemented at an earlier stage and is checked",
- "in tests 138. Blacklisted for debug builds since",
- "these builds have duplicate classes checks which",
- "punt to interpreter"]
- },
- {
"tests": "138-duplicate-classes-check",
"variant": "ndebug",
"description": ["Turned on for debug builds since debug builds have",
diff --git a/test/run-test b/test/run-test
index 044f63f..e6c2480 100755
--- a/test/run-test
+++ b/test/run-test
@@ -92,7 +92,7 @@
export JACK="$JACK -g -cp $JACK_CLASSPATH"
# Allow changing DESUGAR script to something else, or to disable it with DESUGAR=false.
-if [ -z "$DESUGAR"]; then
+if [ -z "$DESUGAR" ]; then
export DESUGAR="$ANDROID_BUILD_TOP/art/tools/desugar.sh"
fi
diff --git a/tools/ahat/src/ObjectHandler.java b/tools/ahat/src/ObjectHandler.java
index d6f1faa..cc55b7a 100644
--- a/tools/ahat/src/ObjectHandler.java
+++ b/tools/ahat/src/ObjectHandler.java
@@ -21,13 +21,15 @@
import com.android.ahat.heapdump.AhatClassObj;
import com.android.ahat.heapdump.AhatInstance;
import com.android.ahat.heapdump.AhatSnapshot;
-import com.android.ahat.heapdump.Diff;
+import com.android.ahat.heapdump.DiffFields;
+import com.android.ahat.heapdump.DiffedFieldValue;
import com.android.ahat.heapdump.FieldValue;
import com.android.ahat.heapdump.PathElement;
import com.android.ahat.heapdump.Site;
import com.android.ahat.heapdump.Value;
import java.io.IOException;
import java.util.Collection;
+import java.util.Collections;
import java.util.List;
import java.util.Objects;
@@ -108,13 +110,9 @@
private static void printClassInstanceFields(Doc doc, Query query, AhatClassInstance inst) {
doc.section("Fields");
AhatInstance base = inst.getBaseline();
- List<FieldValue> fields = inst.getInstanceFields();
- if (!base.isPlaceHolder()) {
- Diff.fields(fields, base.asClassInstance().getInstanceFields());
- }
- SubsetSelector<FieldValue> selector = new SubsetSelector(query, INSTANCE_FIELDS_ID, fields);
- printFields(doc, inst != base && !base.isPlaceHolder(), selector.selected());
- selector.render(doc);
+ printFields(doc, query, INSTANCE_FIELDS_ID, !base.isPlaceHolder(),
+ inst.asClassInstance().getInstanceFields(),
+ base.isPlaceHolder() ? null : base.asClassInstance().getInstanceFields());
}
private static void printArrayElements(Doc doc, Query query, AhatArrayInstance array) {
@@ -145,36 +143,61 @@
selector.render(doc);
}
- private static void printFields(Doc doc, boolean diff, List<FieldValue> fields) {
+ /**
+ * Helper function for printing static or instance fields.
+ * @param id - id to use for the field subset selector.
+ * @param diff - whether a diff should be shown for the fields.
+ * @param current - the fields to show.
+ * @param baseline - the baseline fields to diff against if diff is true,
+ * ignored otherwise.
+ */
+ private static void printFields(Doc doc, Query query, String id, boolean diff,
+ List<FieldValue> current, List<FieldValue> baseline) {
+
+ if (!diff) {
+ // To avoid having to special case when diff is disabled, always diff
+ // the fields, but diff against an empty list.
+ baseline = Collections.emptyList();
+ }
+
+ List<DiffedFieldValue> fields = DiffFields.diff(current, baseline);
+ SubsetSelector<DiffedFieldValue> selector = new SubsetSelector(query, id, fields);
+
doc.table(
new Column("Type"),
new Column("Name"),
new Column("Value"),
new Column("Δ", Column.Align.LEFT, diff));
- for (FieldValue field : fields) {
- Value current = field.getValue();
- DocString value;
- if (field.isPlaceHolder()) {
- value = DocString.removed("del");
- } else {
- value = Summarizer.summarize(current);
- }
+ for (DiffedFieldValue field : selector.selected()) {
+ Value previous = Value.getBaseline(field.baseline);
+ DocString was = DocString.text("was ");
+ was.append(Summarizer.summarize(previous));
+ switch (field.status) {
+ case ADDED:
+ doc.row(DocString.text(field.type),
+ DocString.text(field.name),
+ Summarizer.summarize(field.current),
+ DocString.added("new"));
+ break;
- DocString delta = new DocString();
- FieldValue basefield = field.getBaseline();
- if (basefield.isPlaceHolder()) {
- delta.append(DocString.added("new"));
- } else {
- Value previous = Value.getBaseline(basefield.getValue());
- if (!Objects.equals(current, previous)) {
- delta.append("was ");
- delta.append(Summarizer.summarize(previous));
- }
+ case MATCHED:
+ doc.row(DocString.text(field.type),
+ DocString.text(field.name),
+ Summarizer.summarize(field.current),
+ Objects.equals(field.current, previous) ? new DocString() : was);
+ break;
+
+ case DELETED:
+ doc.row(DocString.text(field.type),
+ DocString.text(field.name),
+ DocString.removed("del"),
+ was);
+ break;
}
- doc.row(DocString.text(field.getType()), DocString.text(field.getName()), value, delta);
}
doc.end();
+ selector.render(doc);
}
private static void printClassInfo(Doc doc, Query query, AhatClassObj clsobj) {
@@ -188,13 +211,9 @@
doc.section("Static Fields");
AhatInstance base = clsobj.getBaseline();
- List<FieldValue> fields = clsobj.getStaticFieldValues();
- if (!base.isPlaceHolder()) {
- Diff.fields(fields, base.asClassObj().getStaticFieldValues());
- }
- SubsetSelector<FieldValue> selector = new SubsetSelector(query, STATIC_FIELDS_ID, fields);
- printFields(doc, clsobj != base && !base.isPlaceHolder(), selector.selected());
- selector.render(doc);
+ printFields(doc, query, STATIC_FIELDS_ID, !base.isPlaceHolder(),
+ clsobj.getStaticFieldValues(),
+ base.isPlaceHolder() ? null : base.asClassObj().getStaticFieldValues());
}
private static void printReferences(Doc doc, Query query, AhatInstance inst) {
diff --git a/tools/ahat/src/heapdump/AhatClassInstance.java b/tools/ahat/src/heapdump/AhatClassInstance.java
index c10d604..158de52 100644
--- a/tools/ahat/src/heapdump/AhatClassInstance.java
+++ b/tools/ahat/src/heapdump/AhatClassInstance.java
@@ -54,8 +54,8 @@
@Override public Value getField(String fieldName) {
for (FieldValue field : mFieldValues) {
- if (fieldName.equals(field.getName())) {
- return field.getValue();
+ if (fieldName.equals(field.name)) {
+ return field.value;
}
}
return null;
diff --git a/tools/ahat/src/heapdump/Diff.java b/tools/ahat/src/heapdump/Diff.java
index 943e6e6..489f709 100644
--- a/tools/ahat/src/heapdump/Diff.java
+++ b/tools/ahat/src/heapdump/Diff.java
@@ -336,48 +336,4 @@
placeholder.getBaseline().getSite().getBaseline().addPlaceHolderInstance(placeholder);
}
}
-
- /**
- * Diff two lists of field values.
- * PlaceHolder objects are added to the given lists as needed to ensure
- * every FieldValue in A ends up with a corresponding FieldValue in B.
- */
- public static void fields(List<FieldValue> a, List<FieldValue> b) {
- // Fields with the same name and type are considered matching fields.
- // For simplicity, we assume the matching fields are in the same order in
- // both A and B, though some fields may be added or removed in either
- // list. If our assumption is wrong, in the worst case the quality of the
- // field diff is poor.
-
- for (int i = 0; i < a.size(); i++) {
- FieldValue afield = a.get(i);
- afield.setBaseline(null);
-
- // Find the matching field in B, if any.
- for (int j = i; j < b.size(); j++) {
- FieldValue bfield = b.get(j);
- if (afield.getName().equals(bfield.getName())
- && afield.getType().equals(bfield.getType())) {
- // We found the matching field in B.
- // Assume fields i, ..., j-1 in B have no match in A.
- for ( ; i < j; i++) {
- a.add(i, FieldValue.newPlaceHolderFieldValue(b.get(i)));
- }
-
- afield.setBaseline(bfield);
- bfield.setBaseline(afield);
- break;
- }
- }
-
- if (afield.getBaseline() == null) {
- b.add(i, FieldValue.newPlaceHolderFieldValue(afield));
- }
- }
-
- // All remaining fields in B are unmatched by any in A.
- for (int i = a.size(); i < b.size(); i++) {
- a.add(i, FieldValue.newPlaceHolderFieldValue(b.get(i)));
- }
- }
}
diff --git a/tools/ahat/src/heapdump/DiffFields.java b/tools/ahat/src/heapdump/DiffFields.java
new file mode 100644
index 0000000..dd73456
--- /dev/null
+++ b/tools/ahat/src/heapdump/DiffFields.java
@@ -0,0 +1,85 @@
+/*
+ * Copyright (C) 2017 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.ahat.heapdump;
+
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.Comparator;
+import java.util.List;
+
+/**
+ * This class contains a routine for diffing two collections of static or
+ * instance fields.
+ */
+public class DiffFields {
+ /**
+ * Return the result of diffing two collections of field values.
+ * The input collections 'current' and 'baseline' are not modified by this function.
+ */
+ public static List<DiffedFieldValue> diff(Collection<FieldValue> current,
+ Collection<FieldValue> baseline) {
+ List<FieldValue> currentSorted = new ArrayList<FieldValue>(current);
+ Collections.sort(currentSorted, FOR_DIFF);
+
+ List<FieldValue> baselineSorted = new ArrayList<FieldValue>(baseline);
+ Collections.sort(baselineSorted, FOR_DIFF);
+
+ // Merge the two lists to form the diffed list of fields.
+ List<DiffedFieldValue> diffed = new ArrayList<DiffedFieldValue>();
+ int currentPos = 0;
+ int baselinePos = 0;
+
+ while (currentPos < currentSorted.size() && baselinePos < baselineSorted.size()) {
+ FieldValue currentField = currentSorted.get(currentPos);
+ FieldValue baselineField = baselineSorted.get(baselinePos);
+ int compare = FOR_DIFF.compare(currentField, baselineField);
+ if (compare < 0) {
+ diffed.add(DiffedFieldValue.added(currentField));
+ currentPos++;
+ } else if (compare == 0) {
+ diffed.add(DiffedFieldValue.matched(currentField, baselineField));
+ currentPos++;
+ baselinePos++;
+ } else {
+ diffed.add(DiffedFieldValue.deleted(baselineField));
+ baselinePos++;
+ }
+ }
+
+ while (currentPos < currentSorted.size()) {
+ FieldValue currentField = currentSorted.get(currentPos);
+ diffed.add(DiffedFieldValue.added(currentField));
+ currentPos++;
+ }
+
+ while (baselinePos < baselineSorted.size()) {
+ FieldValue baselineField = baselineSorted.get(baselinePos);
+ diffed.add(DiffedFieldValue.deleted(baselineField));
+ baselinePos++;
+ }
+ return diffed;
+ }
+
+ /**
+ * Comparator used for sorting fields for the purposes of diff.
+ * Fields with the same name and type are considered comparable, so we sort
+ * by field name and type.
+ */
+ private static final Comparator<FieldValue> FOR_DIFF
+ = new Sort.WithPriority(Sort.FIELD_VALUE_BY_NAME, Sort.FIELD_VALUE_BY_TYPE);
+}
diff --git a/tools/ahat/src/heapdump/DiffedFieldValue.java b/tools/ahat/src/heapdump/DiffedFieldValue.java
new file mode 100644
index 0000000..e2dcf3e
--- /dev/null
+++ b/tools/ahat/src/heapdump/DiffedFieldValue.java
@@ -0,0 +1,101 @@
+/*
+ * Copyright (C) 2017 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.ahat.heapdump;
+
+import java.util.Objects;
+
+/** DiffedFieldValue is used by the DiffedField class to return the result of
+ * diffing two collections of fields.
+ */
+public class DiffedFieldValue {
+ public final String name;
+ public final String type;
+ public final Value current;
+ public final Value baseline;
+
+ public final Status status;
+
+ public static enum Status {
+ ADDED, // The current field has no matching baseline value.
+ MATCHED, // The current field has a matching baseline value.
+ DELETED // The baseline field has no matching current value.
+ };
+
+ /**
+ * Return a DiffedFieldValue where there is both a current and baseline.
+ */
+ public static DiffedFieldValue matched(FieldValue current, FieldValue baseline) {
+ return new DiffedFieldValue(current.name,
+ current.type,
+ current.value,
+ baseline.value,
+ Status.MATCHED);
+ }
+
+ /**
+ * Return a DiffedFieldValue where there is no baseline.
+ */
+ public static DiffedFieldValue added(FieldValue current) {
+ return new DiffedFieldValue(current.name, current.type, current.value, null, Status.ADDED);
+ }
+
+ /**
+ * Return a DiffedFieldValue where there is no current.
+ */
+ public static DiffedFieldValue deleted(FieldValue baseline) {
+ return new DiffedFieldValue(baseline.name, baseline.type, null, baseline.value, Status.DELETED);
+ }
+
+ private DiffedFieldValue(String name, String type, Value current, Value baseline, Status status) {
+ this.name = name;
+ this.type = type;
+ this.current = current;
+ this.baseline = baseline;
+ this.status = status;
+ }
+
+ @Override
+ public boolean equals(Object otherObject) {
+ if (otherObject instanceof DiffedFieldValue) {
+ DiffedFieldValue other = (DiffedFieldValue)otherObject;
+ return name.equals(other.name)
+ && type.equals(other.type)
+ && Objects.equals(current, other.current)
+ && Objects.equals(baseline, other.baseline)
+ && Objects.equals(status, other.status);
+ }
+ return false;
+ }
+
+ @Override
+ public String toString() {
+ switch (status) {
+ case ADDED:
+ return "(" + name + " " + type + " +" + current + ")";
+
+ case MATCHED:
+ return "(" + name + " " + type + " " + current + " " + baseline + ")";
+
+ case DELETED:
+ return "(" + name + " " + type + " -" + baseline + ")";
+
+ default:
+ // There are no other members.
+ throw new AssertionError("unsupported enum member");
+ }
+ }
+}
diff --git a/tools/ahat/src/heapdump/FieldValue.java b/tools/ahat/src/heapdump/FieldValue.java
index 3f65cd3..6d72595 100644
--- a/tools/ahat/src/heapdump/FieldValue.java
+++ b/tools/ahat/src/heapdump/FieldValue.java
@@ -16,68 +16,14 @@
package com.android.ahat.heapdump;
-public class FieldValue implements Diffable<FieldValue> {
- private final String mName;
- private final String mType;
- private final Value mValue;
- private FieldValue mBaseline;
- private final boolean mIsPlaceHolder;
+public class FieldValue {
+ public final String name;
+ public final String type;
+ public final Value value;
public FieldValue(String name, String type, Value value) {
- mName = name;
- mType = type;
- mValue = value;
- mBaseline = this;
- mIsPlaceHolder = false;
- }
-
- /**
- * Construct a place holder FieldValue
- */
- private FieldValue(FieldValue baseline) {
- mName = baseline.mName;
- mType = baseline.mType;
- mValue = Value.getBaseline(baseline.mValue);
- mBaseline = baseline;
- mIsPlaceHolder = true;
- }
-
- static FieldValue newPlaceHolderFieldValue(FieldValue baseline) {
- FieldValue field = new FieldValue(baseline);
- baseline.setBaseline(field);
- return field;
- }
-
- /**
- * Returns the name of the field.
- */
- public String getName() {
- return mName;
- }
-
- /**
- * Returns a description of the type of the field.
- */
- public String getType() {
- return mType;
- }
-
- /**
- * Returns the value of this field.
- */
- public Value getValue() {
- return mValue;
- }
-
- public void setBaseline(FieldValue baseline) {
- mBaseline = baseline;
- }
-
- @Override public FieldValue getBaseline() {
- return mBaseline;
- }
-
- @Override public boolean isPlaceHolder() {
- return mIsPlaceHolder;
+ this.name = name;
+ this.type = type;
+ this.value = value;
}
}
diff --git a/tools/ahat/src/heapdump/Sort.java b/tools/ahat/src/heapdump/Sort.java
index 0745803..efe0d6b 100644
--- a/tools/ahat/src/heapdump/Sort.java
+++ b/tools/ahat/src/heapdump/Sort.java
@@ -200,5 +200,27 @@
return aName.compareTo(bName);
}
};
+
+ /**
+ * Compare FieldValue by field name.
+ */
+ public static final Comparator<FieldValue> FIELD_VALUE_BY_NAME
+ = new Comparator<FieldValue>() {
+ @Override
+ public int compare(FieldValue a, FieldValue b) {
+ return a.name.compareTo(b.name);
+ }
+ };
+
+ /**
+ * Compare FieldValue by type name.
+ */
+ public static final Comparator<FieldValue> FIELD_VALUE_BY_TYPE
+ = new Comparator<FieldValue>() {
+ @Override
+ public int compare(FieldValue a, FieldValue b) {
+ return a.type.compareTo(b.type);
+ }
+ };
}
diff --git a/tools/ahat/src/heapdump/Value.java b/tools/ahat/src/heapdump/Value.java
index 6b2d38f..c1f3022 100644
--- a/tools/ahat/src/heapdump/Value.java
+++ b/tools/ahat/src/heapdump/Value.java
@@ -28,7 +28,7 @@
* The Object must either be a boxed Java primitive type or a subclass of
* AhatInstance. The object must not be null.
*/
- Value(Object object) {
+ public Value(Object object) {
// TODO: Check that the Object is either an AhatSnapshot or boxed Java
// primitive type?
assert object != null;
diff --git a/tools/ahat/test/DiffFieldsTest.java b/tools/ahat/test/DiffFieldsTest.java
new file mode 100644
index 0000000..6abdd47
--- /dev/null
+++ b/tools/ahat/test/DiffFieldsTest.java
@@ -0,0 +1,181 @@
+/*
+ * Copyright (C) 2016 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.ahat;
+
+import com.android.ahat.heapdump.DiffFields;
+import com.android.ahat.heapdump.DiffedFieldValue;
+import com.android.ahat.heapdump.FieldValue;
+import com.android.ahat.heapdump.Value;
+import java.util.ArrayList;
+import java.util.List;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNull;
+
+public class DiffFieldsTest {
+ @Test
+ public void normalMatchedDiffedFieldValues() {
+ FieldValue normal1 = new FieldValue("name", "type", new Value(1));
+ FieldValue normal2 = new FieldValue("name", "type", new Value(2));
+
+ DiffedFieldValue x = DiffedFieldValue.matched(normal1, normal2);
+ assertEquals("name", x.name);
+ assertEquals("type", x.type);
+ assertEquals(new Value(1), x.current);
+ assertEquals(new Value(2), x.baseline);
+ assertEquals(DiffedFieldValue.Status.MATCHED, x.status);
+ }
+
+ @Test
+ public void nulledMatchedDiffedFieldValues() {
+ FieldValue normal = new FieldValue("name", "type", new Value(1));
+ FieldValue nulled = new FieldValue("name", "type", null);
+
+ DiffedFieldValue x = DiffedFieldValue.matched(normal, nulled);
+ assertEquals("name", x.name);
+ assertEquals("type", x.type);
+ assertEquals(new Value(1), x.current);
+ assertNull(x.baseline);
+ assertEquals(DiffedFieldValue.Status.MATCHED, x.status);
+
+ DiffedFieldValue y = DiffedFieldValue.matched(nulled, normal);
+ assertEquals("name", y.name);
+ assertEquals("type", y.type);
+ assertNull(y.current);
+ assertEquals(new Value(1), y.baseline);
+ assertEquals(DiffedFieldValue.Status.MATCHED, y.status);
+ }
+
+ @Test
+ public void normalAddedDiffedFieldValues() {
+ FieldValue normal = new FieldValue("name", "type", new Value(1));
+
+ DiffedFieldValue x = DiffedFieldValue.added(normal);
+ assertEquals("name", x.name);
+ assertEquals("type", x.type);
+ assertEquals(new Value(1), x.current);
+ assertEquals(DiffedFieldValue.Status.ADDED, x.status);
+ }
+
+ @Test
+ public void nulledAddedDiffedFieldValues() {
+ FieldValue nulled = new FieldValue("name", "type", null);
+
+ DiffedFieldValue x = DiffedFieldValue.added(nulled);
+ assertEquals("name", x.name);
+ assertEquals("type", x.type);
+ assertNull(x.current);
+ assertEquals(DiffedFieldValue.Status.ADDED, x.status);
+ }
+
+ @Test
+ public void normalDeletedDiffedFieldValues() {
+ FieldValue normal = new FieldValue("name", "type", new Value(1));
+
+ DiffedFieldValue x = DiffedFieldValue.deleted(normal);
+ assertEquals("name", x.name);
+ assertEquals("type", x.type);
+ assertEquals(new Value(1), x.baseline);
+ assertEquals(DiffedFieldValue.Status.DELETED, x.status);
+ }
+
+ @Test
+ public void nulledDeletedDiffedFieldValues() {
+ FieldValue nulled = new FieldValue("name", "type", null);
+
+ DiffedFieldValue x = DiffedFieldValue.deleted(nulled);
+ assertEquals("name", x.name);
+ assertEquals("type", x.type);
+ assertNull(x.baseline);
+ assertEquals(DiffedFieldValue.Status.DELETED, x.status);
+ }
+
+ @Test
+ public void basicDiff() {
+ List<FieldValue> a = new ArrayList<FieldValue>();
+ a.add(new FieldValue("n0", "t0", null));
+ a.add(new FieldValue("n2", "t2", null));
+ a.add(new FieldValue("n3", "t3", null));
+ a.add(new FieldValue("n4", "t4", null));
+ a.add(new FieldValue("n5", "t5", null));
+ a.add(new FieldValue("n6", "t6", null));
+
+ List<FieldValue> b = new ArrayList<FieldValue>();
+ b.add(new FieldValue("n0", "t0", null));
+ b.add(new FieldValue("n1", "t1", null));
+ b.add(new FieldValue("n2", "t2", null));
+ b.add(new FieldValue("n3", "t3", null));
+ b.add(new FieldValue("n5", "t5", null));
+ b.add(new FieldValue("n6", "t6", null));
+ b.add(new FieldValue("n7", "t7", null));
+
+ // Note: The expected result makes assumptions about the implementation of
+ // field diff to match the order of the returned fields. If the
+ // implementation changes, this test may need to be generalized to accept
+ // the new implementation.
+ List<DiffedFieldValue> expected = new ArrayList<DiffedFieldValue>();
+ expected.add(DiffedFieldValue.matched(a.get(0), b.get(0)));
+ expected.add(DiffedFieldValue.deleted(b.get(1)));
+ expected.add(DiffedFieldValue.matched(a.get(1), b.get(2)));
+ expected.add(DiffedFieldValue.matched(a.get(2), b.get(3)));
+ expected.add(DiffedFieldValue.added(a.get(3)));
+ expected.add(DiffedFieldValue.matched(a.get(4), b.get(4)));
+ expected.add(DiffedFieldValue.matched(a.get(5), b.get(5)));
+ expected.add(DiffedFieldValue.deleted(b.get(6)));
+
+ List<DiffedFieldValue> diffed = DiffFields.diff(a, b);
+ assertEquals(expected, diffed);
+ }
+
+ @Test
+ public void reorderedDiff() {
+ List<FieldValue> a = new ArrayList<FieldValue>();
+ a.add(new FieldValue("n0", "t0", null));
+ a.add(new FieldValue("n1", "t1", null));
+ a.add(new FieldValue("n2", "t2", null));
+ a.add(new FieldValue("n3", "t3", null));
+ a.add(new FieldValue("n4", "t4", null));
+ a.add(new FieldValue("n5", "t5", null));
+ a.add(new FieldValue("n6", "t6", null));
+
+ List<FieldValue> b = new ArrayList<FieldValue>();
+ b.add(new FieldValue("n4", "t4", null));
+ b.add(new FieldValue("n1", "t1", null));
+ b.add(new FieldValue("n3", "t3", null));
+ b.add(new FieldValue("n0", "t0", null));
+ b.add(new FieldValue("n5", "t5", null));
+ b.add(new FieldValue("n2", "t2", null));
+ b.add(new FieldValue("n6", "t6", null));
+
+ // Note: The expected result makes assumptions about the implementation of
+ // field diff to match the order of the returned fields. If the
+ // implementation changes, this test may need to be generalized to accept
+ // the new implementation.
+ List<DiffedFieldValue> expected = new ArrayList<DiffedFieldValue>();
+ expected.add(DiffedFieldValue.matched(a.get(0), b.get(3)));
+ expected.add(DiffedFieldValue.matched(a.get(1), b.get(1)));
+ expected.add(DiffedFieldValue.matched(a.get(2), b.get(5)));
+ expected.add(DiffedFieldValue.matched(a.get(3), b.get(2)));
+ expected.add(DiffedFieldValue.matched(a.get(4), b.get(0)));
+ expected.add(DiffedFieldValue.matched(a.get(5), b.get(4)));
+ expected.add(DiffedFieldValue.matched(a.get(6), b.get(6)));
+
+ List<DiffedFieldValue> diffed = DiffFields.diff(a, b);
+ assertEquals(expected, diffed);
+ }
+}
diff --git a/tools/ahat/test/DiffTest.java b/tools/ahat/test/DiffTest.java
index 52b6b7b..d0349fd 100644
--- a/tools/ahat/test/DiffTest.java
+++ b/tools/ahat/test/DiffTest.java
@@ -20,7 +20,6 @@
import com.android.ahat.heapdump.AhatInstance;
import com.android.ahat.heapdump.AhatSnapshot;
import com.android.ahat.heapdump.Diff;
-import com.android.ahat.heapdump.FieldValue;
import com.android.tools.perflib.heap.hprof.HprofClassDump;
import com.android.tools.perflib.heap.hprof.HprofConstant;
import com.android.tools.perflib.heap.hprof.HprofDumpRecord;
@@ -129,35 +128,4 @@
// Diffing should not crash.
Diff.snapshots(snapshot, snapshot);
}
-
- @Test
- public void diffFields() {
- List<FieldValue> a = new ArrayList<FieldValue>();
- a.add(new FieldValue("n0", "t0", null));
- a.add(new FieldValue("n2", "t2", null));
- a.add(new FieldValue("n3", "t3", null));
- a.add(new FieldValue("n4", "t4", null));
- a.add(new FieldValue("n5", "t5", null));
- a.add(new FieldValue("n6", "t6", null));
-
- List<FieldValue> b = new ArrayList<FieldValue>();
- b.add(new FieldValue("n0", "t0", null));
- b.add(new FieldValue("n1", "t1", null));
- b.add(new FieldValue("n2", "t2", null));
- b.add(new FieldValue("n3", "t3", null));
- b.add(new FieldValue("n5", "t5", null));
- b.add(new FieldValue("n6", "t6", null));
- b.add(new FieldValue("n7", "t7", null));
-
- Diff.fields(a, b);
- assertEquals(8, a.size());
- assertEquals(8, b.size());
- for (int i = 0; i < 8; i++) {
- assertEquals(a.get(i), b.get(i).getBaseline());
- assertEquals(b.get(i), a.get(i).getBaseline());
- }
- assertTrue(a.get(1).isPlaceHolder());
- assertTrue(a.get(7).isPlaceHolder());
- assertTrue(b.get(4).isPlaceHolder());
- }
}
diff --git a/tools/ahat/test/TestDump.java b/tools/ahat/test/TestDump.java
index ceb7346..3dce2dc 100644
--- a/tools/ahat/test/TestDump.java
+++ b/tools/ahat/test/TestDump.java
@@ -118,9 +118,9 @@
private Value getDumpedValue(String name, AhatSnapshot snapshot) {
AhatClassObj main = snapshot.findClass("Main");
AhatInstance stuff = null;
- for (FieldValue fields : main.getStaticFieldValues()) {
- if ("stuff".equals(fields.getName())) {
- stuff = fields.getValue().asAhatInstance();
+ for (FieldValue field : main.getStaticFieldValues()) {
+ if ("stuff".equals(field.name)) {
+ stuff = field.value.asAhatInstance();
}
}
return stuff.getField(name);
diff --git a/tools/ahat/test/Tests.java b/tools/ahat/test/Tests.java
index c7e9b18..a95788e 100644
--- a/tools/ahat/test/Tests.java
+++ b/tools/ahat/test/Tests.java
@@ -22,6 +22,7 @@
public static void main(String[] args) {
if (args.length == 0) {
args = new String[]{
+ "com.android.ahat.DiffFieldsTest",
"com.android.ahat.DiffTest",
"com.android.ahat.InstanceTest",
"com.android.ahat.NativeAllocationTest",
diff --git a/tools/libcore_failures.txt b/tools/libcore_failures.txt
index f340fa1..8a4c2df 100644
--- a/tools/libcore_failures.txt
+++ b/tools/libcore_failures.txt
@@ -216,5 +216,12 @@
bug: 62528691,
modes: [device],
names: ["libcore.java.util.TimeZoneTest#testSetDefaultRace"]
+},
+{
+ description: "Repeated annotations do not work in javac (OpenJDK8), fixed in OpenJDK9.
+ Blacklisted to support javac/dx build (b/36902714)",
+ result: EXEC_FAILED,
+ bug: 62408076,
+ names: ["libcore.java.lang.reflect.annotations.AnnotatedElementParameterTest#testImplicitConstructorParameters_singleAnnotation"]
}
]
