Merge "ART: Throw on soft verify failure in InitializeClass()."
diff --git a/compiler/dex/dex_to_dex_compiler.cc b/compiler/dex/dex_to_dex_compiler.cc
index ad9a30f..fe99eaa 100644
--- a/compiler/dex/dex_to_dex_compiler.cc
+++ b/compiler/dex/dex_to_dex_compiler.cc
@@ -376,9 +376,7 @@
DCHECK_EQ(inst->Opcode(), Instruction::RETURN_VOID);
if (unit_.IsConstructor()) {
// Are we compiling a non clinit constructor which needs a barrier ?
- if (!unit_.IsStatic() &&
- driver_.RequiresConstructorBarrier(Thread::Current(), unit_.GetDexFile(),
- unit_.GetClassDefIndex())) {
+ if (!unit_.IsStatic() && unit_.RequiresConstructorBarrier()) {
return;
}
}
diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc
index df6e8a8..8c276bb 100644
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -254,7 +254,6 @@
verification_results_(verification_results),
compiler_(Compiler::Create(this, compiler_kind)),
compiler_kind_(compiler_kind),
- requires_constructor_barrier_lock_("constructor barrier lock"),
image_classes_(std::move(image_classes)),
number_of_soft_verifier_failures_(0),
had_hard_verifier_failure_(false),
@@ -1580,18 +1579,6 @@
self->ClearException();
}
-bool CompilerDriver::RequiresConstructorBarrier(const DexFile& dex_file,
- uint16_t class_def_idx) const {
- ClassAccessor accessor(dex_file, class_def_idx);
- // We require a constructor barrier if there are final instance fields.
- for (const ClassAccessor::Field& field : accessor.GetInstanceFields()) {
- if (field.IsFinal()) {
- return true;
- }
- }
- return false;
-}
-
class ResolveClassFieldsAndMethodsVisitor : public CompilationVisitor {
public:
explicit ResolveClassFieldsAndMethodsVisitor(const ParallelCompilationManager* manager)
@@ -1635,57 +1622,42 @@
// We want to resolve the methods and fields eagerly.
resolve_fields_and_methods = true;
}
- // If an instance field is final then we need to have a barrier on the return, static final
- // fields are assigned within the lock held for class initialization.
- bool requires_constructor_barrier = false;
- ClassAccessor accessor(dex_file, class_def_index);
- // Optionally resolve fields and methods and figure out if we need a constructor barrier.
- auto method_visitor = [&](const ClassAccessor::Method& method)
- REQUIRES_SHARED(Locks::mutator_lock_) {
- if (resolve_fields_and_methods) {
+ if (resolve_fields_and_methods) {
+ ClassAccessor accessor(dex_file, class_def_index);
+ // Optionally resolve fields and methods and figure out if we need a constructor barrier.
+ auto method_visitor = [&](const ClassAccessor::Method& method)
+ REQUIRES_SHARED(Locks::mutator_lock_) {
ArtMethod* resolved = class_linker->ResolveMethod<ClassLinker::ResolveMode::kNoChecks>(
method.GetIndex(),
dex_cache,
class_loader,
- /* referrer */ nullptr,
+ /*referrer=*/ nullptr,
method.GetInvokeType(class_def.access_flags_));
if (resolved == nullptr) {
CheckAndClearResolveException(soa.Self());
}
- }
- };
- accessor.VisitFieldsAndMethods(
- // static fields
- [&](ClassAccessor::Field& field) REQUIRES_SHARED(Locks::mutator_lock_) {
- if (resolve_fields_and_methods) {
+ };
+ accessor.VisitFieldsAndMethods(
+ // static fields
+ [&](ClassAccessor::Field& field) REQUIRES_SHARED(Locks::mutator_lock_) {
ArtField* resolved = class_linker->ResolveField(
- field.GetIndex(), dex_cache, class_loader, /* is_static */ true);
+ field.GetIndex(), dex_cache, class_loader, /*is_static=*/ true);
if (resolved == nullptr) {
CheckAndClearResolveException(soa.Self());
}
- }
- },
- // instance fields
- [&](ClassAccessor::Field& field) REQUIRES_SHARED(Locks::mutator_lock_) {
- if (field.IsFinal()) {
- // We require a constructor barrier if there are final instance fields.
- requires_constructor_barrier = true;
- }
- if (resolve_fields_and_methods) {
+ },
+ // instance fields
+ [&](ClassAccessor::Field& field) REQUIRES_SHARED(Locks::mutator_lock_) {
ArtField* resolved = class_linker->ResolveField(
- field.GetIndex(), dex_cache, class_loader, /* is_static */ false);
+ field.GetIndex(), dex_cache, class_loader, /*is_static=*/ false);
if (resolved == nullptr) {
CheckAndClearResolveException(soa.Self());
}
- }
- },
- /*direct methods*/ method_visitor,
- /*virtual methods*/ method_visitor);
- manager_->GetCompiler()->SetRequiresConstructorBarrier(self,
- &dex_file,
- class_def_index,
- requires_constructor_barrier);
+ },
+ /*direct_method_visitor=*/ method_visitor,
+ /*virtual_method_visitor=*/ method_visitor);
+ }
}
private:
@@ -2832,31 +2804,6 @@
return is_system_class;
}
-void CompilerDriver::SetRequiresConstructorBarrier(Thread* self,
- const DexFile* dex_file,
- uint16_t class_def_index,
- bool requires) {
- WriterMutexLock mu(self, requires_constructor_barrier_lock_);
- requires_constructor_barrier_.emplace(ClassReference(dex_file, class_def_index), requires);
-}
-
-bool CompilerDriver::RequiresConstructorBarrier(Thread* self,
- const DexFile* dex_file,
- uint16_t class_def_index) {
- ClassReference class_ref(dex_file, class_def_index);
- {
- ReaderMutexLock mu(self, requires_constructor_barrier_lock_);
- auto it = requires_constructor_barrier_.find(class_ref);
- if (it != requires_constructor_barrier_.end()) {
- return it->second;
- }
- }
- WriterMutexLock mu(self, requires_constructor_barrier_lock_);
- const bool requires = RequiresConstructorBarrier(*dex_file, class_def_index);
- requires_constructor_barrier_.emplace(class_ref, requires);
- return requires;
-}
-
std::string CompilerDriver::GetMemoryUsageString(bool extended) const {
std::ostringstream oss;
const gc::Heap* const heap = Runtime::Current()->GetHeap();
diff --git a/compiler/driver/compiler_driver.h b/compiler/driver/compiler_driver.h
index 9a83e55..f42e555 100644
--- a/compiler/driver/compiler_driver.h
+++ b/compiler/driver/compiler_driver.h
@@ -151,50 +151,6 @@
void AddCompiledMethod(const MethodReference& method_ref, CompiledMethod* const compiled_method);
CompiledMethod* RemoveCompiledMethod(const MethodReference& method_ref);
- void SetRequiresConstructorBarrier(Thread* self,
- const DexFile* dex_file,
- uint16_t class_def_index,
- bool requires)
- REQUIRES(!requires_constructor_barrier_lock_);
-
- // Do the <init> methods for this class require a constructor barrier (prior to the return)?
- // The answer is "yes", if and only if this class has any instance final fields.
- // (This must not be called for any non-<init> methods; the answer would be "no").
- //
- // ---
- //
- // JLS 17.5.1 "Semantics of final fields" mandates that all final fields are frozen at the end
- // of the invoked constructor. The constructor barrier is a conservative implementation means of
- // enforcing the freezes happen-before the object being constructed is observable by another
- // thread.
- //
- // Note: This question only makes sense for instance constructors;
- // static constructors (despite possibly having finals) never need
- // a barrier.
- //
- // JLS 12.4.2 "Detailed Initialization Procedure" approximately describes
- // class initialization as:
- //
- // lock(class.lock)
- // class.state = initializing
- // unlock(class.lock)
- //
- // invoke <clinit>
- //
- // lock(class.lock)
- // class.state = initialized
- // unlock(class.lock) <-- acts as a release
- //
- // The last operation in the above example acts as an atomic release
- // for any stores in <clinit>, which ends up being stricter
- // than what a constructor barrier needs.
- //
- // See also QuasiAtomic::ThreadFenceForConstructor().
- bool RequiresConstructorBarrier(Thread* self,
- const DexFile* dex_file,
- uint16_t class_def_index)
- REQUIRES(!requires_constructor_barrier_lock_);
-
// Resolve compiling method's class. Returns null on failure.
ObjPtr<mirror::Class> ResolveCompilingMethodsClass(const ScopedObjectAccess& soa,
Handle<mirror::DexCache> dex_cache,
@@ -407,20 +363,12 @@
void FreeThreadPools();
void CheckThreadPools();
- bool RequiresConstructorBarrier(const DexFile& dex_file, uint16_t class_def_idx) const;
-
const CompilerOptions* const compiler_options_;
VerificationResults* const verification_results_;
std::unique_ptr<Compiler> compiler_;
Compiler::Kind compiler_kind_;
- // All class references that require constructor barriers. If the class reference is not in the
- // set then the result has not yet been computed.
- mutable ReaderWriterMutex requires_constructor_barrier_lock_ DEFAULT_MUTEX_ACQUIRED_AFTER;
- std::map<ClassReference, bool> requires_constructor_barrier_
- GUARDED_BY(requires_constructor_barrier_lock_);
-
// All class references that this compiler has compiled. Indexed by class defs.
using ClassStateTable = AtomicDexRefMap<ClassReference, ClassStatus>;
ClassStateTable compiled_classes_;
diff --git a/compiler/driver/compiler_options_map.def b/compiler/driver/compiler_options_map.def
index 1ec34ec..a593240 100644
--- a/compiler/driver/compiler_options_map.def
+++ b/compiler/driver/compiler_options_map.def
@@ -52,7 +52,7 @@
COMPILER_OPTIONS_KEY (double, TopKProfileThreshold)
COMPILER_OPTIONS_KEY (bool, AbortOnHardVerifierFailure)
COMPILER_OPTIONS_KEY (bool, AbortOnSoftVerifierFailure)
-COMPILER_OPTIONS_KEY (bool, ResolveStartupConstStrings, kIsDebugBuild)
+COMPILER_OPTIONS_KEY (bool, ResolveStartupConstStrings, false)
COMPILER_OPTIONS_KEY (std::string, DumpInitFailures)
COMPILER_OPTIONS_KEY (std::string, DumpCFG)
COMPILER_OPTIONS_KEY (Unit, DumpCFGAppend)
diff --git a/compiler/driver/dex_compilation_unit.cc b/compiler/driver/dex_compilation_unit.cc
index 654eccd..e5a6f0e 100644
--- a/compiler/driver/dex_compilation_unit.cc
+++ b/compiler/driver/dex_compilation_unit.cc
@@ -16,10 +16,14 @@
#include "dex_compilation_unit.h"
+#include "art_field.h"
#include "base/utils.h"
+#include "dex/class_accessor-inl.h"
#include "dex/code_item_accessors-inl.h"
#include "dex/descriptors_names.h"
+#include "mirror/class-inl.h"
#include "mirror/dex_cache.h"
+#include "scoped_thread_state_change-inl.h"
namespace art {
@@ -53,4 +57,32 @@
return symbol_;
}
+bool DexCompilationUnit::RequiresConstructorBarrier() const {
+ // Constructor barriers are applicable only for <init> methods.
+ DCHECK(!IsStatic());
+ DCHECK(IsConstructor());
+
+ // We require a constructor barrier if there are final instance fields.
+ if (GetCompilingClass().GetReference() != nullptr && !GetCompilingClass().IsNull()) {
+ // Decoding class data can be slow, so iterate over fields of the compiling class if resolved.
+ ScopedObjectAccess soa(Thread::Current());
+ ObjPtr<mirror::Class> compiling_class = GetCompilingClass().Get();
+ for (size_t i = 0, size = compiling_class->NumInstanceFields(); i != size; ++i) {
+ ArtField* field = compiling_class->GetInstanceField(i);
+ if (field->IsFinal()) {
+ return true;
+ }
+ }
+ } else {
+ // Iterate over field definitions in the class data.
+ ClassAccessor accessor(*GetDexFile(), GetClassDefIndex());
+ for (const ClassAccessor::Field& field : accessor.GetInstanceFields()) {
+ if (field.IsFinal()) {
+ return true;
+ }
+ }
+ }
+ return false;
+}
+
} // namespace art
diff --git a/compiler/driver/dex_compilation_unit.h b/compiler/driver/dex_compilation_unit.h
index 9d9c218..757f0e7 100644
--- a/compiler/driver/dex_compilation_unit.h
+++ b/compiler/driver/dex_compilation_unit.h
@@ -123,6 +123,41 @@
return compiling_class_;
}
+ // Does this <init> method require a constructor barrier (prior to the return)?
+ // The answer is "yes", if and only if the class has any instance final fields.
+ // (This must not be called for any non-<init> methods; the answer would be "no").
+ //
+ // ---
+ //
+ // JLS 17.5.1 "Semantics of final fields" mandates that all final fields are frozen at the end
+ // of the invoked constructor. The constructor barrier is a conservative implementation means of
+ // enforcing the freezes happen-before the object being constructed is observable by another
+ // thread.
+ //
+ // Note: This question only makes sense for instance constructors;
+ // static constructors (despite possibly having finals) never need
+ // a barrier.
+ //
+ // JLS 12.4.2 "Detailed Initialization Procedure" approximately describes
+ // class initialization as:
+ //
+ // lock(class.lock)
+ // class.state = initializing
+ // unlock(class.lock)
+ //
+ // invoke <clinit>
+ //
+ // lock(class.lock)
+ // class.state = initialized
+ // unlock(class.lock) <-- acts as a release
+ //
+ // The last operation in the above example acts as an atomic release
+ // for any stores in <clinit>, which ends up being stricter
+ // than what a constructor barrier needs.
+ //
+ // See also QuasiAtomic::ThreadFenceForConstructor().
+ bool RequiresConstructorBarrier() const;
+
private:
const Handle<mirror::ClassLoader> class_loader_;
diff --git a/compiler/optimizing/builder.cc b/compiler/optimizing/builder.cc
index a1a5692..64aa1b9 100644
--- a/compiler/optimizing/builder.cc
+++ b/compiler/optimizing/builder.cc
@@ -21,6 +21,7 @@
#include "base/bit_vector-inl.h"
#include "base/logging.h"
#include "block_builder.h"
+#include "code_generator.h"
#include "data_type-inl.h"
#include "dex/verified_method.h"
#include "driver/compiler_options.h"
@@ -40,7 +41,6 @@
const CodeItemDebugInfoAccessor& accessor,
const DexCompilationUnit* dex_compilation_unit,
const DexCompilationUnit* outer_compilation_unit,
- CompilerDriver* driver,
CodeGenerator* code_generator,
OptimizingCompilerStats* compiler_stats,
ArrayRef<const uint8_t> interpreter_metadata,
@@ -50,7 +50,6 @@
code_item_accessor_(accessor),
dex_compilation_unit_(dex_compilation_unit),
outer_compilation_unit_(outer_compilation_unit),
- compiler_driver_(driver),
code_generator_(code_generator),
compilation_stats_(compiler_stats),
interpreter_metadata_(interpreter_metadata),
@@ -67,19 +66,18 @@
code_item_accessor_(accessor),
dex_compilation_unit_(dex_compilation_unit),
outer_compilation_unit_(nullptr),
- compiler_driver_(nullptr),
code_generator_(nullptr),
compilation_stats_(nullptr),
handles_(handles),
return_type_(return_type) {}
bool HGraphBuilder::SkipCompilation(size_t number_of_branches) {
- if (compiler_driver_ == nullptr) {
- // Note that the compiler driver is null when unit testing.
+ if (code_generator_ == nullptr) {
+ // Note that the codegen is null when unit testing.
return false;
}
- const CompilerOptions& compiler_options = compiler_driver_->GetCompilerOptions();
+ const CompilerOptions& compiler_options = code_generator_->GetCompilerOptions();
CompilerFilter::Filter compiler_filter = compiler_options.GetCompilerFilter();
if (compiler_filter == CompilerFilter::kEverything) {
return false;
@@ -131,7 +129,6 @@
return_type_,
dex_compilation_unit_,
outer_compilation_unit_,
- compiler_driver_,
code_generator_,
interpreter_metadata_,
compilation_stats_,
@@ -203,7 +200,6 @@
return_type_,
dex_compilation_unit_,
outer_compilation_unit_,
- compiler_driver_,
code_generator_,
interpreter_metadata_,
compilation_stats_,
diff --git a/compiler/optimizing/builder.h b/compiler/optimizing/builder.h
index 5a1914c..6152740 100644
--- a/compiler/optimizing/builder.h
+++ b/compiler/optimizing/builder.h
@@ -22,7 +22,6 @@
#include "dex/code_item_accessors.h"
#include "dex/dex_file-inl.h"
#include "dex/dex_file.h"
-#include "driver/compiler_driver.h"
#include "nodes.h"
namespace art {
@@ -38,7 +37,6 @@
const CodeItemDebugInfoAccessor& accessor,
const DexCompilationUnit* dex_compilation_unit,
const DexCompilationUnit* outer_compilation_unit,
- CompilerDriver* driver,
CodeGenerator* code_generator,
OptimizingCompilerStats* compiler_stats,
ArrayRef<const uint8_t> interpreter_metadata,
@@ -70,7 +68,6 @@
// The compilation unit of the enclosing method being compiled.
const DexCompilationUnit* const outer_compilation_unit_;
- CompilerDriver* const compiler_driver_;
CodeGenerator* const code_generator_;
OptimizingCompilerStats* const compilation_stats_;
diff --git a/compiler/optimizing/code_generator.h b/compiler/optimizing/code_generator.h
index 3f56078..39966ff 100644
--- a/compiler/optimizing/code_generator.h
+++ b/compiler/optimizing/code_generator.h
@@ -59,7 +59,6 @@
class Assembler;
class CodeGenerator;
-class CompilerDriver;
class CompilerOptions;
class StackMapStream;
class ParallelMoveResolver;
diff --git a/compiler/optimizing/inliner.cc b/compiler/optimizing/inliner.cc
index f9e9abb..f56f9cb 100644
--- a/compiler/optimizing/inliner.cc
+++ b/compiler/optimizing/inliner.cc
@@ -1807,7 +1807,6 @@
code_item_accessor,
&dex_compilation_unit,
&outer_compilation_unit_,
- compiler_driver_,
codegen_,
inline_stats_,
resolved_method->GetQuickenedInfo(),
diff --git a/compiler/optimizing/instruction_builder.cc b/compiler/optimizing/instruction_builder.cc
index 5af8796..e9b5b5a 100644
--- a/compiler/optimizing/instruction_builder.cc
+++ b/compiler/optimizing/instruction_builder.cc
@@ -25,7 +25,6 @@
#include "data_type-inl.h"
#include "dex/bytecode_utils.h"
#include "dex/dex_instruction-inl.h"
-#include "driver/compiler_driver.h"
#include "driver/dex_compilation_unit.h"
#include "driver/compiler_options.h"
#include "imtable-inl.h"
@@ -48,7 +47,6 @@
DataType::Type return_type,
const DexCompilationUnit* dex_compilation_unit,
const DexCompilationUnit* outer_compilation_unit,
- CompilerDriver* compiler_driver,
CodeGenerator* code_generator,
ArrayRef<const uint8_t> interpreter_metadata,
OptimizingCompilerStats* compiler_stats,
@@ -62,7 +60,6 @@
return_type_(return_type),
block_builder_(block_builder),
ssa_builder_(ssa_builder),
- compiler_driver_(compiler_driver),
code_generator_(code_generator),
dex_compilation_unit_(dex_compilation_unit),
outer_compilation_unit_(outer_compilation_unit),
@@ -711,20 +708,18 @@
// Does the method being compiled need any constructor barriers being inserted?
// (Always 'false' for methods that aren't <init>.)
-static bool RequiresConstructorBarrier(const DexCompilationUnit* cu, CompilerDriver* driver) {
+static bool RequiresConstructorBarrier(const DexCompilationUnit* cu) {
// Can be null in unit tests only.
if (UNLIKELY(cu == nullptr)) {
return false;
}
- Thread* self = Thread::Current();
- return cu->IsConstructor()
- && !cu->IsStatic()
- // RequiresConstructorBarrier must only be queried for <init> methods;
- // it's effectively "false" for every other method.
- //
- // See CompilerDriver::RequiresConstructBarrier for more explanation.
- && driver->RequiresConstructorBarrier(self, cu->GetDexFile(), cu->GetClassDefIndex());
+ // Constructor barriers are applicable only for <init> methods.
+ if (LIKELY(!cu->IsConstructor() || cu->IsStatic())) {
+ return false;
+ }
+
+ return cu->RequiresConstructorBarrier();
}
// Returns true if `block` has only one successor which starts at the next
@@ -770,7 +765,7 @@
// Only <init> (which is a return-void) could possibly have a constructor fence.
// This may insert additional redundant constructor fences from the super constructors.
// TODO: remove redundant constructor fences (b/36656456).
- if (RequiresConstructorBarrier(dex_compilation_unit_, compiler_driver_)) {
+ if (RequiresConstructorBarrier(dex_compilation_unit_)) {
// Compiling instance constructor.
DCHECK_STREQ("<init>", graph_->GetMethodName());
@@ -784,7 +779,7 @@
}
AppendInstruction(new (allocator_) HReturnVoid(dex_pc));
} else {
- DCHECK(!RequiresConstructorBarrier(dex_compilation_unit_, compiler_driver_));
+ DCHECK(!RequiresConstructorBarrier(dex_compilation_unit_));
HInstruction* value = LoadLocal(instruction.VRegA(), type);
AppendInstruction(new (allocator_) HReturn(value, dex_pc));
}
@@ -881,8 +876,8 @@
// The back-end code generator relies on this check in order to ensure that it will not
// attempt to read the dex_cache with a dex_method_index that is not from the correct
// dex_file. If we didn't do this check then the dex_method_index will not be updated in the
- // builder, which means that the code-generator (and compiler driver during sharpening and
- // inliner, maybe) might invoke an incorrect method.
+ // builder, which means that the code-generator (and sharpening and inliner, maybe)
+ // might invoke an incorrect method.
// TODO: The actual method could still be referenced in the current dex file, so we
// could try locating it.
// TODO: Remove the dex_file restriction.
diff --git a/compiler/optimizing/instruction_builder.h b/compiler/optimizing/instruction_builder.h
index 21afd11..d701445 100644
--- a/compiler/optimizing/instruction_builder.h
+++ b/compiler/optimizing/instruction_builder.h
@@ -34,7 +34,6 @@
class ArtField;
class ArtMethod;
class CodeGenerator;
-class CompilerDriver;
class DexCompilationUnit;
class HBasicBlockBuilder;
class Instruction;
@@ -59,7 +58,6 @@
DataType::Type return_type,
const DexCompilationUnit* dex_compilation_unit,
const DexCompilationUnit* outer_compilation_unit,
- CompilerDriver* compiler_driver,
CodeGenerator* code_generator,
ArrayRef<const uint8_t> interpreter_metadata,
OptimizingCompilerStats* compiler_stats,
@@ -307,8 +305,6 @@
HBasicBlockBuilder* const block_builder_;
SsaBuilder* const ssa_builder_;
- CompilerDriver* const compiler_driver_;
-
CodeGenerator* const code_generator_;
// The compilation unit of the current method being compiled. Note that
diff --git a/compiler/optimizing/intrinsics.h b/compiler/optimizing/intrinsics.h
index 8245453..5bd1122 100644
--- a/compiler/optimizing/intrinsics.h
+++ b/compiler/optimizing/intrinsics.h
@@ -24,7 +24,6 @@
namespace art {
-class CompilerDriver;
class DexFile;
// Positive floating-point infinities.
diff --git a/compiler/optimizing/nodes.h b/compiler/optimizing/nodes.h
index 6ebe89e..2124380 100644
--- a/compiler/optimizing/nodes.h
+++ b/compiler/optimizing/nodes.h
@@ -7402,7 +7402,7 @@
// }
//
// See also:
-// * CompilerDriver::RequiresConstructorBarrier
+// * DexCompilationUnit::RequiresConstructorBarrier
// * QuasiAtomic::ThreadFenceForConstructor
//
class HConstructorFence final : public HVariableInputSizeInstruction {
diff --git a/compiler/optimizing/optimizing_compiler.cc b/compiler/optimizing/optimizing_compiler.cc
index 38daaaa..4f495b6 100644
--- a/compiler/optimizing/optimizing_compiler.cc
+++ b/compiler/optimizing/optimizing_compiler.cc
@@ -870,7 +870,6 @@
code_item_accessor,
&dex_compilation_unit,
&dex_compilation_unit,
- compiler_driver,
codegen.get(),
compilation_stats_.get(),
interpreter_metadata,
@@ -991,7 +990,6 @@
CodeItemDebugInfoAccessor(), // Null code item.
&dex_compilation_unit,
&dex_compilation_unit,
- compiler_driver,
codegen.get(),
compilation_stats_.get(),
/* interpreter_metadata */ ArrayRef<const uint8_t>(),
diff --git a/libartbase/arch/instruction_set.cc b/libartbase/arch/instruction_set.cc
index a187663..d47f936 100644
--- a/libartbase/arch/instruction_set.cc
+++ b/libartbase/arch/instruction_set.cc
@@ -105,18 +105,7 @@
UNREACHABLE();
}
-#if !defined(ART_STACK_OVERFLOW_GAP_arm) || !defined(ART_STACK_OVERFLOW_GAP_arm64) || \
- !defined(ART_STACK_OVERFLOW_GAP_mips) || !defined(ART_STACK_OVERFLOW_GAP_mips64) || \
- !defined(ART_STACK_OVERFLOW_GAP_x86) || !defined(ART_STACK_OVERFLOW_GAP_x86_64)
-#error "Missing defines for stack overflow gap"
-#endif
-
-static constexpr size_t kArmStackOverflowReservedBytes = ART_STACK_OVERFLOW_GAP_arm;
-static constexpr size_t kArm64StackOverflowReservedBytes = ART_STACK_OVERFLOW_GAP_arm64;
-static constexpr size_t kMipsStackOverflowReservedBytes = ART_STACK_OVERFLOW_GAP_mips;
-static constexpr size_t kMips64StackOverflowReservedBytes = ART_STACK_OVERFLOW_GAP_mips64;
-static constexpr size_t kX86StackOverflowReservedBytes = ART_STACK_OVERFLOW_GAP_x86;
-static constexpr size_t kX86_64StackOverflowReservedBytes = ART_STACK_OVERFLOW_GAP_x86_64;
+namespace instruction_set_details {
static_assert(IsAligned<kPageSize>(kArmStackOverflowReservedBytes), "ARM gap not page aligned");
static_assert(IsAligned<kPageSize>(kArm64StackOverflowReservedBytes), "ARM64 gap not page aligned");
@@ -144,32 +133,10 @@
static_assert(ART_FRAME_SIZE_LIMIT < kX86_64StackOverflowReservedBytes,
"Frame size limit too large");
-size_t GetStackOverflowReservedBytes(InstructionSet isa) {
- switch (isa) {
- case InstructionSet::kArm: // Intentional fall-through.
- case InstructionSet::kThumb2:
- return kArmStackOverflowReservedBytes;
+} // namespace instruction_set_details
- case InstructionSet::kArm64:
- return kArm64StackOverflowReservedBytes;
-
- case InstructionSet::kMips:
- return kMipsStackOverflowReservedBytes;
-
- case InstructionSet::kMips64:
- return kMips64StackOverflowReservedBytes;
-
- case InstructionSet::kX86:
- return kX86StackOverflowReservedBytes;
-
- case InstructionSet::kX86_64:
- return kX86_64StackOverflowReservedBytes;
-
- case InstructionSet::kNone:
- LOG(FATAL) << "kNone has no stack overflow size";
- UNREACHABLE();
- }
- LOG(FATAL) << "Unknown instruction set" << isa;
+NO_RETURN void GetStackOverflowReservedBytesFailure(const char* error_msg) {
+ LOG(FATAL) << error_msg;
UNREACHABLE();
}
diff --git a/libartbase/arch/instruction_set.h b/libartbase/arch/instruction_set.h
index 06bd53a..7e071bd 100644
--- a/libartbase/arch/instruction_set.h
+++ b/libartbase/arch/instruction_set.h
@@ -226,7 +226,53 @@
InstructionSetAbort(isa);
}
-size_t GetStackOverflowReservedBytes(InstructionSet isa);
+namespace instruction_set_details {
+
+#if !defined(ART_STACK_OVERFLOW_GAP_arm) || !defined(ART_STACK_OVERFLOW_GAP_arm64) || \
+ !defined(ART_STACK_OVERFLOW_GAP_mips) || !defined(ART_STACK_OVERFLOW_GAP_mips64) || \
+ !defined(ART_STACK_OVERFLOW_GAP_x86) || !defined(ART_STACK_OVERFLOW_GAP_x86_64)
+#error "Missing defines for stack overflow gap"
+#endif
+
+static constexpr size_t kArmStackOverflowReservedBytes = ART_STACK_OVERFLOW_GAP_arm;
+static constexpr size_t kArm64StackOverflowReservedBytes = ART_STACK_OVERFLOW_GAP_arm64;
+static constexpr size_t kMipsStackOverflowReservedBytes = ART_STACK_OVERFLOW_GAP_mips;
+static constexpr size_t kMips64StackOverflowReservedBytes = ART_STACK_OVERFLOW_GAP_mips64;
+static constexpr size_t kX86StackOverflowReservedBytes = ART_STACK_OVERFLOW_GAP_x86;
+static constexpr size_t kX86_64StackOverflowReservedBytes = ART_STACK_OVERFLOW_GAP_x86_64;
+
+NO_RETURN void GetStackOverflowReservedBytesFailure(const char* error_msg);
+
+} // namespace instruction_set_details
+
+ALWAYS_INLINE
+constexpr size_t GetStackOverflowReservedBytes(InstructionSet isa) {
+ switch (isa) {
+ case InstructionSet::kArm: // Intentional fall-through.
+ case InstructionSet::kThumb2:
+ return instruction_set_details::kArmStackOverflowReservedBytes;
+
+ case InstructionSet::kArm64:
+ return instruction_set_details::kArm64StackOverflowReservedBytes;
+
+ case InstructionSet::kMips:
+ return instruction_set_details::kMipsStackOverflowReservedBytes;
+
+ case InstructionSet::kMips64:
+ return instruction_set_details::kMips64StackOverflowReservedBytes;
+
+ case InstructionSet::kX86:
+ return instruction_set_details::kX86StackOverflowReservedBytes;
+
+ case InstructionSet::kX86_64:
+ return instruction_set_details::kX86_64StackOverflowReservedBytes;
+
+ case InstructionSet::kNone:
+ instruction_set_details::GetStackOverflowReservedBytesFailure(
+ "kNone has no stack overflow size");
+ }
+ instruction_set_details::GetStackOverflowReservedBytesFailure("Unknown instruction set");
+}
// The following definitions create return types for two word-sized entities that will be passed
// in registers so that memory operations for the interface trampolines can be avoided. The entities
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index c9257b9..9ba52c4 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -1495,18 +1495,44 @@
intern_table->AddImageStringsToTable(space, [&](InternTable::UnorderedSet& interns)
REQUIRES_SHARED(Locks::mutator_lock_)
REQUIRES(Locks::intern_table_lock_) {
+ const size_t non_boot_image_strings = intern_table->CountInterns(
+ /*visit_boot_images=*/false,
+ /*visit_non_boot_images=*/true);
VLOG(image) << "AppImage:stringsInInternTableSize = " << interns.size();
- for (auto it = interns.begin(); it != interns.end(); ) {
- ObjPtr<mirror::String> string = it->Read();
- ObjPtr<mirror::String> existing = intern_table->LookupWeakLocked(string);
- if (existing == nullptr) {
- existing = intern_table->LookupStrongLocked(string);
+ VLOG(image) << "AppImage:nonBootImageInternStrings = " << non_boot_image_strings;
+ // Visit the smaller of the two sets to compute the intersection.
+ if (interns.size() < non_boot_image_strings) {
+ for (auto it = interns.begin(); it != interns.end(); ) {
+ ObjPtr<mirror::String> string = it->Read();
+ ObjPtr<mirror::String> existing = intern_table->LookupWeakLocked(string);
+ if (existing == nullptr) {
+ existing = intern_table->LookupStrongLocked(string);
+ }
+ if (existing != nullptr) {
+ intern_remap.Put(string.Ptr(), existing.Ptr());
+ it = interns.erase(it);
+ } else {
+ ++it;
+ }
}
- if (existing != nullptr) {
- intern_remap.Put(string.Ptr(), existing.Ptr());
- it = interns.erase(it);
- } else {
- ++it;
+ } else {
+ intern_table->VisitInterns([&](const GcRoot<mirror::String>& root)
+ REQUIRES_SHARED(Locks::mutator_lock_)
+ REQUIRES(Locks::intern_table_lock_) {
+ auto it = interns.find(root);
+ if (it != interns.end()) {
+ ObjPtr<mirror::String> existing = root.Read();
+ intern_remap.Put(it->Read(), existing.Ptr());
+ it = interns.erase(it);
+ }
+ }, /*visit_boot_images=*/false, /*visit_non_boot_images=*/true);
+ }
+ // Sanity check to ensure correctness.
+ if (kIsDebugBuild) {
+ for (GcRoot<mirror::String>& root : interns) {
+ ObjPtr<mirror::String> string = root.Read();
+ CHECK(intern_table->LookupWeakLocked(string) == nullptr) << string->ToModifiedUtf8();
+ CHECK(intern_table->LookupStrongLocked(string) == nullptr) << string->ToModifiedUtf8();
}
}
});
diff --git a/runtime/handle.h b/runtime/handle.h
index 18e503d..b13c43e 100644
--- a/runtime/handle.h
+++ b/runtime/handle.h
@@ -62,8 +62,9 @@
return down_cast<T*>(reference_->AsMirrorPtr());
}
- ALWAYS_INLINE bool IsNull() const REQUIRES_SHARED(Locks::mutator_lock_) {
- return Get() == nullptr;
+ ALWAYS_INLINE bool IsNull() const {
+ // It's safe to null-check it without a read barrier.
+ return reference_->IsNull();
}
ALWAYS_INLINE jobject ToJObject() const REQUIRES_SHARED(Locks::mutator_lock_) {
diff --git a/runtime/intern_table-inl.h b/runtime/intern_table-inl.h
index 3c7da09..6fc53e9 100644
--- a/runtime/intern_table-inl.h
+++ b/runtime/intern_table-inl.h
@@ -77,7 +77,7 @@
bool visit_boot_images,
bool visit_non_boot_images) {
auto visit_tables = [&](std::vector<Table::InternalTable>& tables)
- REQUIRES_SHARED(Locks::mutator_lock_) {
+ NO_THREAD_SAFETY_ANALYSIS {
for (Table::InternalTable& table : tables) {
// Determine if we want to visit the table based on the flags..
const bool visit =
@@ -94,6 +94,26 @@
visit_tables(weak_interns_.tables_);
}
+inline size_t InternTable::CountInterns(bool visit_boot_images,
+ bool visit_non_boot_images) const {
+ size_t ret = 0u;
+ auto visit_tables = [&](const std::vector<Table::InternalTable>& tables)
+ NO_THREAD_SAFETY_ANALYSIS {
+ for (const Table::InternalTable& table : tables) {
+ // Determine if we want to visit the table based on the flags..
+ const bool visit =
+ (visit_boot_images && table.IsBootImage()) ||
+ (visit_non_boot_images && !table.IsBootImage());
+ if (visit) {
+ ret += table.set_.size();
+ }
+ }
+ };
+ visit_tables(strong_interns_.tables_);
+ visit_tables(weak_interns_.tables_);
+ return ret;
+}
+
} // namespace art
#endif // ART_RUNTIME_INTERN_TABLE_INL_H_
diff --git a/runtime/intern_table.h b/runtime/intern_table.h
index baeb1a9..165d56c 100644
--- a/runtime/intern_table.h
+++ b/runtime/intern_table.h
@@ -174,6 +174,10 @@
bool visit_non_boot_images)
REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(Locks::intern_table_lock_);
+ // Count the number of intern strings in the table.
+ size_t CountInterns(bool visit_boot_images, bool visit_non_boot_images) const
+ REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(Locks::intern_table_lock_);
+
void DumpForSigQuit(std::ostream& os) const REQUIRES(!Locks::intern_table_lock_);
void BroadcastForNewInterns();
diff --git a/test/004-ThreadStress/src-art/Main.java b/test/004-ThreadStress/src-art/Main.java
index 3a89f4f..b8bfafb 100644
--- a/test/004-ThreadStress/src-art/Main.java
+++ b/test/004-ThreadStress/src-art/Main.java
@@ -26,6 +26,7 @@
import java.util.Map;
import java.util.Set;
import java.util.concurrent.Semaphore;
+import java.util.concurrent.locks.LockSupport;
// Run on host with:
// javac ThreadTest.java && java ThreadStress && rm *.class
@@ -251,6 +252,31 @@
}
}
+ private final static class TimedPark extends Operation {
+ private final static int SLEEP_TIME = 100;
+
+ public TimedPark() {}
+
+ @Override
+ public boolean perform() {
+ LockSupport.parkNanos(this, 100*1000000);
+ return true;
+ }
+ }
+
+ private final static class UnparkAllThreads extends Operation {
+ public UnparkAllThreads() {}
+
+ @Override
+ public boolean perform() {
+ Set<Thread> threads = Thread.getAllStackTraces().keySet();
+ for (Thread candidate : threads) {
+ LockSupport.unpark(candidate);
+ }
+ return true;
+ }
+ }
+
private final static class SyncAndWork extends Operation {
private final Object lock;
@@ -320,7 +346,9 @@
frequencyMap.put(new NonMovingAlloc(), 0.025); // 5/200
frequencyMap.put(new StackTrace(), 0.1); // 20/200
frequencyMap.put(new Exit(), 0.225); // 45/200
- frequencyMap.put(new Sleep(), 0.125); // 25/200
+ frequencyMap.put(new Sleep(), 0.125); // 15/200
+ frequencyMap.put(new TimedPark(), 0.025); // 5/200
+ frequencyMap.put(new UnparkAllThreads(), 0.025); // 5/200
frequencyMap.put(new TimedWait(lock), 0.05); // 10/200
frequencyMap.put(new Wait(lock), 0.075); // 15/200
frequencyMap.put(new QueuedWait(semaphore), 0.05); // 10/200
@@ -341,9 +369,11 @@
private final static Map<Operation, Double> createLockFrequencyMap(Object lock) {
Map<Operation, Double> frequencyMap = new HashMap<Operation, Double>();
frequencyMap.put(new Sleep(), 0.2); // 40/200
- frequencyMap.put(new TimedWait(lock), 0.2); // 40/200
- frequencyMap.put(new Wait(lock), 0.2); // 40/200
+ frequencyMap.put(new TimedWait(lock), 0.1); // 20/200
+ frequencyMap.put(new Wait(lock), 0.1); // 20/200
frequencyMap.put(new SyncAndWork(lock), 0.4); // 80/200
+ frequencyMap.put(new TimedPark(), 0.1); // 20/200
+ frequencyMap.put(new UnparkAllThreads(), 0.1); // 20/200
return frequencyMap;
}