runtime: Minor cleanup and extra comments around interpreter
Change-Id: I24c0b261de8cf737babd9d01bf679482d48c8bc9
diff --git a/compiler/dex/verified_method.cc b/compiler/dex/verified_method.cc
index ac7a4a7..6d48598 100644
--- a/compiler/dex/verified_method.cc
+++ b/compiler/dex/verified_method.cc
@@ -98,7 +98,7 @@
}
size_t ref_bitmap_bytes = RoundUp(ref_bitmap_bits, kBitsPerByte) / kBitsPerByte;
// There are 2 bytes to encode the number of entries.
- if (num_entries >= 65536) {
+ if (num_entries > std::numeric_limits<uint16_t>::max()) {
LOG(WARNING) << "Cannot encode GC map for method with " << num_entries << " entries: "
<< PrettyMethod(method_verifier->GetMethodReference().dex_method_index,
*method_verifier->GetMethodReference().dex_file);
diff --git a/compiler/dex/verified_method.h b/compiler/dex/verified_method.h
index 242e3df..07f9a9b 100644
--- a/compiler/dex/verified_method.h
+++ b/compiler/dex/verified_method.h
@@ -120,7 +120,7 @@
DequickenMap dequicken_map_;
SafeCastSet safe_cast_set_;
- bool has_verification_failures_;
+ bool has_verification_failures_ = false;
// Copy of mapping generated by verifier of dex PCs of string init invocations
// to the set of other registers that the receiver has been copied into.
diff --git a/runtime/dex_file.h b/runtime/dex_file.h
index d017601..7ac264a 100644
--- a/runtime/dex_file.h
+++ b/runtime/dex_file.h
@@ -264,13 +264,18 @@
// Raw code_item.
struct CodeItem {
- uint16_t registers_size_;
- uint16_t ins_size_;
- uint16_t outs_size_;
- uint16_t tries_size_;
- uint32_t debug_info_off_; // file offset to debug info stream
+ uint16_t registers_size_; // the number of registers used by this code
+ // (locals + parameters)
+ uint16_t ins_size_; // the number of words of incoming arguments to the method
+ // that this code is for
+ uint16_t outs_size_; // the number of words of outgoing argument space required
+ // by this code for method invocation
+ uint16_t tries_size_; // the number of try_items for this instance. If non-zero,
+ // then these appear as the tries array just after the
+ // insns in this instance.
+ uint32_t debug_info_off_; // file offset to debug info stream
uint32_t insns_size_in_code_units_; // size of the insns array, in 2 byte code units
- uint16_t insns_[1];
+ uint16_t insns_[1]; // actual array of bytecode.
private:
DISALLOW_COPY_AND_ASSIGN(CodeItem);
diff --git a/runtime/interpreter/interpreter_common.cc b/runtime/interpreter/interpreter_common.cc
index 86a79ce..0f6f788 100644
--- a/runtime/interpreter/interpreter_common.cc
+++ b/runtime/interpreter/interpreter_common.cc
@@ -450,10 +450,13 @@
static inline void AssignRegister(ShadowFrame* new_shadow_frame, const ShadowFrame& shadow_frame,
size_t dest_reg, size_t src_reg)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
- // If both register locations contains the same value, the register probably holds a reference.
// Uint required, so that sign extension does not make this wrong on 64b systems
uint32_t src_value = shadow_frame.GetVReg(src_reg);
mirror::Object* o = shadow_frame.GetVRegReference<kVerifyNone>(src_reg);
+
+ // If both register locations contains the same value, the register probably holds a reference.
+ // Note: As an optimization, non-moving collectors leave a stale reference value
+ // in the references array even after the original vreg was overwritten to a non-reference.
if (src_value == reinterpret_cast<uintptr_t>(o)) {
new_shadow_frame->SetVRegReference(dest_reg, o);
} else {
diff --git a/runtime/interpreter/interpreter_switch_impl.cc b/runtime/interpreter/interpreter_switch_impl.cc
index dd7aa40..fcf083c 100644
--- a/runtime/interpreter/interpreter_switch_impl.cc
+++ b/runtime/interpreter/interpreter_switch_impl.cc
@@ -56,7 +56,7 @@
template<bool do_access_check, bool transaction_active>
JValue ExecuteSwitchImpl(Thread* self, const DexFile::CodeItem* code_item,
ShadowFrame& shadow_frame, JValue result_register) {
- bool do_assignability_check = do_access_check;
+ constexpr bool do_assignability_check = do_access_check;
if (UNLIKELY(!shadow_frame.HasReferenceArray())) {
LOG(FATAL) << "Invalid shadow frame for interpreter use";
return JValue();
diff --git a/runtime/mirror/class-inl.h b/runtime/mirror/class-inl.h
index 835b94a..8c9222f 100644
--- a/runtime/mirror/class-inl.h
+++ b/runtime/mirror/class-inl.h
@@ -666,7 +666,7 @@
inline void Class::VisitReferences(mirror::Class* klass, const Visitor& visitor) {
VisitInstanceFieldsReferences<kVisitClass>(klass, visitor);
// Right after a class is allocated, but not yet loaded
- // (kStatusNotReady, see ClassLinkder::LoadClass()), GC may find it
+ // (kStatusNotReady, see ClassLinker::LoadClass()), GC may find it
// and scan it. IsTemp() may call Class::GetAccessFlags() but may
// fail in the DCHECK in Class::GetAccessFlags() because the class
// status is kStatusNotReady. To avoid it, rely on IsResolved()
diff --git a/runtime/mirror/string-inl.h b/runtime/mirror/string-inl.h
index 9f6cd11..8d9c08d 100644
--- a/runtime/mirror/string-inl.h
+++ b/runtime/mirror/string-inl.h
@@ -176,11 +176,13 @@
}
template <bool kIsInstrumented>
-inline String* String::AllocFromCharArray(Thread* self, int32_t array_length,
+inline String* String::AllocFromCharArray(Thread* self, int32_t count,
Handle<CharArray> array, int32_t offset,
gc::AllocatorType allocator_type) {
- SetStringCountAndValueVisitorFromCharArray visitor(array_length, array, offset);
- String* new_string = Alloc<kIsInstrumented>(self, array_length, allocator_type, visitor);
+ // It is a caller error to have a count less than the actual array's size.
+ DCHECK_GE(array->GetLength(), count);
+ SetStringCountAndValueVisitorFromCharArray visitor(count, array, offset);
+ String* new_string = Alloc<kIsInstrumented>(self, count, allocator_type, visitor);
return new_string;
}
diff --git a/runtime/mirror/string.h b/runtime/mirror/string.h
index a8f16d7..af06385 100644
--- a/runtime/mirror/string.h
+++ b/runtime/mirror/string.h
@@ -95,7 +95,7 @@
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
template <bool kIsInstrumented>
- ALWAYS_INLINE static String* AllocFromCharArray(Thread* self, int32_t array_length,
+ ALWAYS_INLINE static String* AllocFromCharArray(Thread* self, int32_t count,
Handle<CharArray> array, int32_t offset,
gc::AllocatorType allocator_type)
SHARED_LOCKS_REQUIRED(Locks::mutator_lock_);
diff --git a/runtime/read_barrier-inl.h b/runtime/read_barrier-inl.h
index d341ee1..8d84c35 100644
--- a/runtime/read_barrier-inl.h
+++ b/runtime/read_barrier-inl.h
@@ -31,7 +31,7 @@
template <typename MirrorType, ReadBarrierOption kReadBarrierOption, bool kMaybeDuringStartup>
inline MirrorType* ReadBarrier::Barrier(
mirror::Object* obj, MemberOffset offset, mirror::HeapReference<MirrorType>* ref_addr) {
- const bool with_read_barrier = kReadBarrierOption == kWithReadBarrier;
+ constexpr bool with_read_barrier = kReadBarrierOption == kWithReadBarrier;
if (with_read_barrier && kUseBakerReadBarrier) {
// The higher bits of the rb ptr, rb_ptr_high_bits (must be zero)
// is used to create artificial data dependency from the is_gray
diff --git a/runtime/runtime_options.def b/runtime/runtime_options.def
index 922334e..4a307d5 100644
--- a/runtime/runtime_options.def
+++ b/runtime/runtime_options.def
@@ -30,6 +30,8 @@
// If a default value is omitted here, T{} is used as the default value, which is
// almost-always the value of the type as if it was memset to all 0.
//
+// Please keep the columns aligned if possible when adding new rows.
+//
// Parse-able keys from the command line.
RUNTIME_OPTIONS_KEY (Unit, Zygote)
@@ -64,9 +66,9 @@
RUNTIME_OPTIONS_KEY (Unit, LowMemoryMode)
RUNTIME_OPTIONS_KEY (bool, UseTLAB, (kUseTlab || kUseReadBarrier))
RUNTIME_OPTIONS_KEY (bool, EnableHSpaceCompactForOOM, true)
-RUNTIME_OPTIONS_KEY (bool, UseJIT, false)
-RUNTIME_OPTIONS_KEY (unsigned int, JITCompileThreshold, jit::Jit::kDefaultCompileThreshold)
-RUNTIME_OPTIONS_KEY (MemoryKiB, JITCodeCacheCapacity, jit::JitCodeCache::kDefaultCapacity)
+RUNTIME_OPTIONS_KEY (bool, UseJIT, false)
+RUNTIME_OPTIONS_KEY (unsigned int, JITCompileThreshold, jit::Jit::kDefaultCompileThreshold)
+RUNTIME_OPTIONS_KEY (MemoryKiB, JITCodeCacheCapacity, jit::JitCodeCache::kDefaultCapacity)
RUNTIME_OPTIONS_KEY (MillisecondsToNanoseconds, \
HSpaceCompactForOOMMinIntervalsMs,\
MsToNs(100 * 1000)) // 100s
@@ -105,9 +107,12 @@
ImageCompilerOptions) // -Ximage-compiler-option ...
RUNTIME_OPTIONS_KEY (bool, Verify, true)
RUNTIME_OPTIONS_KEY (std::string, NativeBridge)
+RUNTIME_OPTIONS_KEY (unsigned int, ZygoteMaxFailedBoots, 10)
+RUNTIME_OPTIONS_KEY (Unit, NoDexFileFallback)
RUNTIME_OPTIONS_KEY (std::string, CpuAbiList)
// Not parse-able from command line, but can be provided explicitly.
+// (Do not add anything here that is defined in ParsedOptions::MakeParser)
RUNTIME_OPTIONS_KEY (const std::vector<const DexFile*>*, \
BootClassPathDexList) // TODO: make unique_ptr
RUNTIME_OPTIONS_KEY (InstructionSet, ImageInstructionSet, kRuntimeISA)
@@ -120,7 +125,5 @@
// We don't call abort(3) by default; see
// Runtime::Abort.
RUNTIME_OPTIONS_KEY (void (*)(), HookAbort, nullptr)
-RUNTIME_OPTIONS_KEY (unsigned int, ZygoteMaxFailedBoots, 10)
-RUNTIME_OPTIONS_KEY (Unit, NoDexFileFallback)
#undef RUNTIME_OPTIONS_KEY
diff --git a/runtime/stack.h b/runtime/stack.h
index 79d2f40..d60714f 100644
--- a/runtime/stack.h
+++ b/runtime/stack.h
@@ -95,6 +95,8 @@
}
~ShadowFrame() {}
+ // TODO(iam): Clean references array up since they're always there,
+ // we don't need to do conditionals.
bool HasReferenceArray() const {
return true;
}
@@ -149,6 +151,9 @@
return *reinterpret_cast<unaligned_double*>(vreg);
}
+ // Look up the reference given its virtual register number.
+ // If this returns non-null then this does not mean the vreg is currently a reference
+ // on non-moving collectors. Check that the raw reg with GetVReg is equal to this if not certain.
template<VerifyObjectFlags kVerifyFlags = kDefaultVerifyFlags>
mirror::Object* GetVRegReference(size_t i) const SHARED_LOCKS_REQUIRED(Locks::mutator_lock_) {
DCHECK_LT(i, NumberOfVRegs());
@@ -283,6 +288,8 @@
ShadowFrame(uint32_t num_vregs, ShadowFrame* link, ArtMethod* method,
uint32_t dex_pc, bool has_reference_array)
: number_of_vregs_(num_vregs), link_(link), method_(method), dex_pc_(dex_pc) {
+ // TODO(iam): Remove this parameter, it's an an artifact of portable removal
+ DCHECK(has_reference_array);
if (has_reference_array) {
memset(vregs_, 0, num_vregs * (sizeof(uint32_t) + sizeof(StackReference<mirror::Object>)));
} else {
@@ -306,6 +313,15 @@
ShadowFrame* link_;
ArtMethod* method_;
uint32_t dex_pc_;
+
+ // This is a two-part array:
+ // - [0..number_of_vregs) holds the raw virtual registers, and each element here is always 4
+ // bytes.
+ // - [number_of_vregs..number_of_vregs*2) holds only reference registers. Each element here is
+ // ptr-sized.
+ // In other words when a primitive is stored in vX, the second (reference) part of the array will
+ // be null. When a reference is stored in vX, the second (reference) part of the array will be a
+ // copy of vX.
uint32_t vregs_[0];
DISALLOW_IMPLICIT_CONSTRUCTORS(ShadowFrame);
