From bf44e0e5281de91f2e38a9378b94ef8c50ad9b23 Mon Sep 17 00:00:00 2001 From: Christina Wadsworth Date: Thu, 18 Aug 2016 10:37:42 -0700 Subject: [PATCH] ART: Implement a fixed size string dex cache Previously, the string dex cache was dex_file->NumStringIds() size, and @ruhler found that only ~1% of that cache was ever getting filled. Since many of these string dex caches were previously 100,000+ indices in length, we're wasting a few hundred KB per app by storing null pointers. The intent of this project was to reduce the space the string dex cache is using, while not regressing on time that much. This is the first of a few CLs, which implements the new fixed size array and disables the compiled code so it always goes slow path. In four other CLs, I implemented a "medium path" that regresses from the previous "fast path" only a bit in assembly in the entrypoints. @vmarko will introduce new compiled code in the future so that we ultimately won't be regressing on time at all. Overall, space savings have been confirmed as on the order of 100 KB per application. A 4-5% slow down in art-opt on Golem, and no noticeable slow down in the interpreter. The opt slow down should be diminished once the new compiled code is introduced. Test: m test-art-host Bug: 20323084 Change-Id: Ic654a1fb9c1ae127dde59290bf36a23edb55ca8e --- compiler/image_writer.cc | 6 ++- compiler/oat_writer.cc | 9 +++- compiler/optimizing/code_generator_arm.cc | 52 +++------------------ compiler/optimizing/code_generator_arm64.cc | 61 +++---------------------- compiler/optimizing/code_generator_mips.cc | 54 +++------------------- compiler/optimizing/code_generator_mips64.cc | 21 +++------ compiler/optimizing/code_generator_x86.cc | 45 +++---------------- compiler/optimizing/code_generator_x86_64.cc | 50 +++------------------ compiler/optimizing/sharpening.cc | 36 ++------------- patchoat/patchoat.cc | 5 ++- runtime/class_linker-inl.h | 19 +++++--- runtime/class_linker.cc | 49 +++++++++++++------- runtime/gc/space/image_space.cc | 4 +- runtime/interpreter/interpreter_common.h | 20 ++++++--- runtime/mirror/class-inl.h | 11 +++-- runtime/mirror/class.h | 7 ++- runtime/mirror/dex_cache-inl.h | 40 +++++++++++------ runtime/mirror/dex_cache.cc | 2 +- runtime/mirror/dex_cache.h | 67 ++++++++++++++++++++++++---- runtime/mirror/dex_cache_test.cc | 4 +- runtime/native/java_lang_DexCache.cc | 4 +- runtime/utils/dex_cache_arrays_layout-inl.h | 19 +++++--- runtime/utils/dex_cache_arrays_layout.h | 2 +- test/Android.run-test.mk | 2 + 24 files changed, 231 insertions(+), 358 deletions(-) diff --git a/compiler/image_writer.cc b/compiler/image_writer.cc index efae4d058..bb459996e 100644 --- a/compiler/image_writer.cc +++ b/compiler/image_writer.cc @@ -52,6 +52,7 @@ #include "mirror/array-inl.h" #include "mirror/class-inl.h" #include "mirror/class_loader.h" +#include "mirror/dex_cache.h" #include "mirror/dex_cache-inl.h" #include "mirror/method.h" #include "mirror/object-inl.h" @@ -1418,6 +1419,9 @@ void ImageWriter::CalculateNewObjectOffsets() { bin_offset = RoundUp(bin_offset, method_alignment); break; } + case kBinDexCacheArray: + bin_offset = RoundUp(bin_offset, DexCacheArraysLayout::Alignment()); + break; case kBinImTable: case kBinIMTConflictTable: { bin_offset = RoundUp(bin_offset, static_cast(target_ptr_size_)); @@ -2034,7 +2038,7 @@ void ImageWriter::FixupDexCache(mirror::DexCache* orig_dex_cache, // 64-bit values here, clearing the top 32 bits for 32-bit targets. The zero-extension is // done by casting to the unsigned type uintptr_t before casting to int64_t, i.e. // static_cast(reinterpret_cast(image_begin_ + offset))). - GcRoot* orig_strings = orig_dex_cache->GetStrings(); + mirror::StringDexCacheType* orig_strings = orig_dex_cache->GetStrings(); if (orig_strings != nullptr) { copy_dex_cache->SetFieldPtrWithSize(mirror::DexCache::StringsOffset(), NativeLocationInImage(orig_strings), diff --git a/compiler/oat_writer.cc b/compiler/oat_writer.cc index 8273b1566..8a809822d 100644 --- a/compiler/oat_writer.cc +++ b/compiler/oat_writer.cc @@ -1189,8 +1189,13 @@ class OatWriter::WriteCodeMethodVisitor : public OatDexMethodVisitor { } mirror::String* GetTargetString(const LinkerPatch& patch) SHARED_REQUIRES(Locks::mutator_lock_) { - mirror::DexCache* dex_cache = GetDexCache(patch.TargetStringDexFile()); - mirror::String* string = dex_cache->GetResolvedString(patch.TargetStringIndex()); + ScopedObjectAccessUnchecked soa(Thread::Current()); + StackHandleScope<1> hs(soa.Self()); + ClassLinker* linker = Runtime::Current()->GetClassLinker(); + Handle dex_cache(hs.NewHandle(GetDexCache(patch.TargetStringDexFile()))); + mirror::String* string = linker->LookupString(*patch.TargetStringDexFile(), + patch.TargetStringIndex(), + dex_cache); DCHECK(string != nullptr); DCHECK(writer_->HasBootImage() || Runtime::Current()->GetHeap()->ObjectIsInBootImageSpace(string)); diff --git a/compiler/optimizing/code_generator_arm.cc b/compiler/optimizing/code_generator_arm.cc index 4c4128c5f..870d1fbd2 100644 --- a/compiler/optimizing/code_generator_arm.cc +++ b/compiler/optimizing/code_generator_arm.cc @@ -5585,55 +5585,15 @@ void InstructionCodeGeneratorARM::VisitLoadString(HLoadString* load) { __ LoadLiteral(out, codegen_->DeduplicateBootImageAddressLiteral(address)); return; // No dex cache slow path. } - case HLoadString::LoadKind::kDexCacheAddress: { - DCHECK_NE(load->GetAddress(), 0u); - uint32_t address = dchecked_integral_cast(load->GetAddress()); - // 16-bit LDR immediate has a 5-bit offset multiplied by the size and that gives - // a 128B range. To try and reduce the number of literals if we load multiple strings, - // simply split the dex cache address to a 128B aligned base loaded from a literal - // and the remaining offset embedded in the load. - static_assert(sizeof(GcRoot) == 4u, "Expected GC root to be 4 bytes."); - DCHECK_ALIGNED(load->GetAddress(), 4u); - constexpr size_t offset_bits = /* encoded bits */ 5 + /* scale */ 2; - uint32_t base_address = address & ~MaxInt(offset_bits); - uint32_t offset = address & MaxInt(offset_bits); - __ LoadLiteral(out, codegen_->DeduplicateDexCacheAddressLiteral(base_address)); - // /* GcRoot */ out = *(base_address + offset) - GenerateGcRootFieldLoad(load, out_loc, out, offset); - break; - } - case HLoadString::LoadKind::kDexCachePcRelative: { - Register base_reg = locations->InAt(0).AsRegister(); - HArmDexCacheArraysBase* base = load->InputAt(0)->AsArmDexCacheArraysBase(); - int32_t offset = load->GetDexCacheElementOffset() - base->GetElementOffset(); - // /* GcRoot */ out = *(dex_cache_arrays_base + offset) - GenerateGcRootFieldLoad(load, out_loc, base_reg, offset); - break; - } - case HLoadString::LoadKind::kDexCacheViaMethod: { - Register current_method = locations->InAt(0).AsRegister(); - - // /* GcRoot */ out = current_method->declaring_class_ - GenerateGcRootFieldLoad( - load, out_loc, current_method, ArtMethod::DeclaringClassOffset().Int32Value()); - // /* GcRoot[] */ out = out->dex_cache_strings_ - __ LoadFromOffset(kLoadWord, out, out, mirror::Class::DexCacheStringsOffset().Int32Value()); - // /* GcRoot */ out = out[string_index] - GenerateGcRootFieldLoad( - load, out_loc, out, CodeGenerator::GetCacheOffset(load->GetStringIndex())); - break; - } default: - LOG(FATAL) << "Unexpected load kind: " << load->GetLoadKind(); - UNREACHABLE(); + break; } - if (!load->IsInDexCache()) { - SlowPathCode* slow_path = new (GetGraph()->GetArena()) LoadStringSlowPathARM(load); - codegen_->AddSlowPath(slow_path); - __ CompareAndBranchIfZero(out, slow_path->GetEntryLabel()); - __ Bind(slow_path->GetExitLabel()); - } + // TODO: Re-add the compiler code to do string dex cache lookup again. + SlowPathCode* slow_path = new (GetGraph()->GetArena()) LoadStringSlowPathARM(load); + codegen_->AddSlowPath(slow_path); + __ b(slow_path->GetEntryLabel()); + __ Bind(slow_path->GetExitLabel()); } static int32_t GetExceptionTlsOffset() { diff --git a/compiler/optimizing/code_generator_arm64.cc b/compiler/optimizing/code_generator_arm64.cc index d95e7df6b..004d42751 100644 --- a/compiler/optimizing/code_generator_arm64.cc +++ b/compiler/optimizing/code_generator_arm64.cc @@ -4195,7 +4195,6 @@ void LocationsBuilderARM64::VisitLoadString(HLoadString* load) { } void InstructionCodeGeneratorARM64::VisitLoadString(HLoadString* load) { - Location out_loc = load->GetLocations()->Out(); Register out = OutputRegister(load); switch (load->GetLoadKind()) { @@ -4231,63 +4230,15 @@ void InstructionCodeGeneratorARM64::VisitLoadString(HLoadString* load) { __ Ldr(out.W(), codegen_->DeduplicateBootImageAddressLiteral(load->GetAddress())); return; // No dex cache slow path. } - case HLoadString::LoadKind::kDexCacheAddress: { - DCHECK_NE(load->GetAddress(), 0u); - // LDR immediate has a 12-bit offset multiplied by the size and for 32-bit loads - // that gives a 16KiB range. To try and reduce the number of literals if we load - // multiple strings, simply split the dex cache address to a 16KiB aligned base - // loaded from a literal and the remaining offset embedded in the load. - static_assert(sizeof(GcRoot) == 4u, "Expected GC root to be 4 bytes."); - DCHECK_ALIGNED(load->GetAddress(), 4u); - constexpr size_t offset_bits = /* encoded bits */ 12 + /* scale */ 2; - uint64_t base_address = load->GetAddress() & ~MaxInt(offset_bits); - uint32_t offset = load->GetAddress() & MaxInt(offset_bits); - __ Ldr(out.X(), codegen_->DeduplicateDexCacheAddressLiteral(base_address)); - // /* GcRoot */ out = *(base_address + offset) - GenerateGcRootFieldLoad(load, out_loc, out.X(), offset); - break; - } - case HLoadString::LoadKind::kDexCachePcRelative: { - // Add ADRP with its PC-relative DexCache access patch. - const DexFile& dex_file = load->GetDexFile(); - uint32_t element_offset = load->GetDexCacheElementOffset(); - vixl::aarch64::Label* adrp_label = - codegen_->NewPcRelativeDexCacheArrayPatch(dex_file, element_offset); - { - SingleEmissionCheckScope guard(GetVIXLAssembler()); - __ Bind(adrp_label); - __ adrp(out.X(), /* offset placeholder */ 0); - } - // Add LDR with its PC-relative DexCache access patch. - vixl::aarch64::Label* ldr_label = - codegen_->NewPcRelativeDexCacheArrayPatch(dex_file, element_offset, adrp_label); - // /* GcRoot */ out = *(base_address + offset) /* PC-relative */ - GenerateGcRootFieldLoad(load, out_loc, out.X(), /* offset placeholder */ 0, ldr_label); - break; - } - case HLoadString::LoadKind::kDexCacheViaMethod: { - Register current_method = InputRegisterAt(load, 0); - // /* GcRoot */ out = current_method->declaring_class_ - GenerateGcRootFieldLoad( - load, out_loc, current_method, ArtMethod::DeclaringClassOffset().Int32Value()); - // /* GcRoot[] */ out = out->dex_cache_strings_ - __ Ldr(out.X(), HeapOperand(out, mirror::Class::DexCacheStringsOffset().Uint32Value())); - // /* GcRoot */ out = out[string_index] - GenerateGcRootFieldLoad( - load, out_loc, out.X(), CodeGenerator::GetCacheOffset(load->GetStringIndex())); - break; - } default: - LOG(FATAL) << "Unexpected load kind: " << load->GetLoadKind(); - UNREACHABLE(); + break; } - if (!load->IsInDexCache()) { - SlowPathCodeARM64* slow_path = new (GetGraph()->GetArena()) LoadStringSlowPathARM64(load); - codegen_->AddSlowPath(slow_path); - __ Cbz(out, slow_path->GetEntryLabel()); - __ Bind(slow_path->GetExitLabel()); - } + // TODO: Re-add the compiler code to do string dex cache lookup again. + SlowPathCodeARM64* slow_path = new (GetGraph()->GetArena()) LoadStringSlowPathARM64(load); + codegen_->AddSlowPath(slow_path); + __ B(slow_path->GetEntryLabel()); + __ Bind(slow_path->GetExitLabel()); } void LocationsBuilderARM64::VisitLongConstant(HLongConstant* constant) { diff --git a/compiler/optimizing/code_generator_mips.cc b/compiler/optimizing/code_generator_mips.cc index 58879bc2f..a7fbc8434 100644 --- a/compiler/optimizing/code_generator_mips.cc +++ b/compiler/optimizing/code_generator_mips.cc @@ -4580,11 +4580,6 @@ void InstructionCodeGeneratorMIPS::VisitLoadString(HLoadString* load) { case HLoadString::LoadKind::kBootImageLinkTimePcRelative: base_or_current_method_reg = isR6 ? ZERO : locations->InAt(0).AsRegister(); break; - // We need an extra register for PC-relative dex cache accesses. - case HLoadString::LoadKind::kDexCachePcRelative: - case HLoadString::LoadKind::kDexCacheViaMethod: - base_or_current_method_reg = locations->InAt(0).AsRegister(); - break; default: base_or_current_method_reg = ZERO; break; @@ -4628,52 +4623,15 @@ void InstructionCodeGeneratorMIPS::VisitLoadString(HLoadString* load) { codegen_->DeduplicateBootImageAddressLiteral(address)); return; // No dex cache slow path. } - case HLoadString::LoadKind::kDexCacheAddress: { - DCHECK_NE(load->GetAddress(), 0u); - uint32_t address = dchecked_integral_cast(load->GetAddress()); - static_assert(sizeof(GcRoot) == 4u, "Expected GC root to be 4 bytes."); - DCHECK_ALIGNED(load->GetAddress(), 4u); - int16_t offset = Low16Bits(address); - uint32_t base_address = address - offset; // This accounts for offset sign extension. - __ Lui(out, High16Bits(base_address)); - // /* GcRoot */ out = *(base_address + offset) - GenerateGcRootFieldLoad(load, out_loc, out, offset); - break; - } - case HLoadString::LoadKind::kDexCachePcRelative: { - HMipsDexCacheArraysBase* base = load->InputAt(0)->AsMipsDexCacheArraysBase(); - int32_t offset = - load->GetDexCacheElementOffset() - base->GetElementOffset() - kDexCacheArrayLwOffset; - // /* GcRoot */ out = *(dex_cache_arrays_base + offset) - GenerateGcRootFieldLoad(load, out_loc, base_or_current_method_reg, offset); - break; - } - case HLoadString::LoadKind::kDexCacheViaMethod: { - // /* GcRoot */ out = current_method->declaring_class_ - GenerateGcRootFieldLoad(load, - out_loc, - base_or_current_method_reg, - ArtMethod::DeclaringClassOffset().Int32Value()); - // /* GcRoot[] */ out = out->dex_cache_strings_ - __ LoadFromOffset(kLoadWord, out, out, mirror::Class::DexCacheStringsOffset().Int32Value()); - // /* GcRoot */ out = out[string_index] - GenerateGcRootFieldLoad(load, - out_loc, - out, - CodeGenerator::GetCacheOffset(load->GetStringIndex())); - break; - } default: - LOG(FATAL) << "Unexpected load kind: " << load->GetLoadKind(); - UNREACHABLE(); + break; } - if (!load->IsInDexCache()) { - SlowPathCodeMIPS* slow_path = new (GetGraph()->GetArena()) LoadStringSlowPathMIPS(load); - codegen_->AddSlowPath(slow_path); - __ Beqz(out, slow_path->GetEntryLabel()); - __ Bind(slow_path->GetExitLabel()); - } + // TODO: Re-add the compiler code to do string dex cache lookup again. + SlowPathCodeMIPS* slow_path = new (GetGraph()->GetArena()) LoadStringSlowPathMIPS(load); + codegen_->AddSlowPath(slow_path); + __ B(slow_path->GetEntryLabel()); + __ Bind(slow_path->GetExitLabel()); } void LocationsBuilderMIPS::VisitLongConstant(HLongConstant* constant) { diff --git a/compiler/optimizing/code_generator_mips64.cc b/compiler/optimizing/code_generator_mips64.cc index 4e7a2728b..4a5755c92 100644 --- a/compiler/optimizing/code_generator_mips64.cc +++ b/compiler/optimizing/code_generator_mips64.cc @@ -3261,22 +3261,11 @@ void LocationsBuilderMIPS64::VisitLoadString(HLoadString* load) { } void InstructionCodeGeneratorMIPS64::VisitLoadString(HLoadString* load) { - LocationSummary* locations = load->GetLocations(); - GpuRegister out = locations->Out().AsRegister(); - GpuRegister current_method = locations->InAt(0).AsRegister(); - __ LoadFromOffset(kLoadUnsignedWord, out, current_method, - ArtMethod::DeclaringClassOffset().Int32Value()); - __ LoadFromOffset(kLoadDoubleword, out, out, mirror::Class::DexCacheStringsOffset().Int32Value()); - __ LoadFromOffset( - kLoadUnsignedWord, out, out, CodeGenerator::GetCacheOffset(load->GetStringIndex())); - // TODO: We will need a read barrier here. - - if (!load->IsInDexCache()) { - SlowPathCodeMIPS64* slow_path = new (GetGraph()->GetArena()) LoadStringSlowPathMIPS64(load); - codegen_->AddSlowPath(slow_path); - __ Beqzc(out, slow_path->GetEntryLabel()); - __ Bind(slow_path->GetExitLabel()); - } + // TODO: Re-add the compiler code to do string dex cache lookup again. + SlowPathCodeMIPS64* slow_path = new (GetGraph()->GetArena()) LoadStringSlowPathMIPS64(load); + codegen_->AddSlowPath(slow_path); + __ Bc(slow_path->GetEntryLabel()); + __ Bind(slow_path->GetExitLabel()); } void LocationsBuilderMIPS64::VisitLongConstant(HLongConstant* constant) { diff --git a/compiler/optimizing/code_generator_x86.cc b/compiler/optimizing/code_generator_x86.cc index 7a561bb4a..0305d6a03 100644 --- a/compiler/optimizing/code_generator_x86.cc +++ b/compiler/optimizing/code_generator_x86.cc @@ -6230,48 +6230,15 @@ void InstructionCodeGeneratorX86::VisitLoadString(HLoadString* load) { codegen_->RecordSimplePatch(); return; // No dex cache slow path. } - case HLoadString::LoadKind::kDexCacheAddress: { - DCHECK_NE(load->GetAddress(), 0u); - uint32_t address = dchecked_integral_cast(load->GetAddress()); - // /* GcRoot */ out = *address - GenerateGcRootFieldLoad(load, out_loc, Address::Absolute(address)); - break; - } - case HLoadString::LoadKind::kDexCachePcRelative: { - Register base_reg = locations->InAt(0).AsRegister(); - uint32_t offset = load->GetDexCacheElementOffset(); - Label* fixup_label = codegen_->NewPcRelativeDexCacheArrayPatch(load->GetDexFile(), offset); - // /* GcRoot */ out = *(base + offset) /* PC-relative */ - GenerateGcRootFieldLoad( - load, out_loc, Address(base_reg, CodeGeneratorX86::kDummy32BitOffset), fixup_label); - break; - } - case HLoadString::LoadKind::kDexCacheViaMethod: { - Register current_method = locations->InAt(0).AsRegister(); - - // /* GcRoot */ out = current_method->declaring_class_ - GenerateGcRootFieldLoad( - load, out_loc, Address(current_method, ArtMethod::DeclaringClassOffset().Int32Value())); - - // /* GcRoot[] */ out = out->dex_cache_strings_ - __ movl(out, Address(out, mirror::Class::DexCacheStringsOffset().Int32Value())); - // /* GcRoot */ out = out[string_index] - GenerateGcRootFieldLoad( - load, out_loc, Address(out, CodeGenerator::GetCacheOffset(load->GetStringIndex()))); - break; - } default: - LOG(FATAL) << "Unexpected load kind: " << load->GetLoadKind(); - UNREACHABLE(); + break; } - if (!load->IsInDexCache()) { - SlowPathCode* slow_path = new (GetGraph()->GetArena()) LoadStringSlowPathX86(load); - codegen_->AddSlowPath(slow_path); - __ testl(out, out); - __ j(kEqual, slow_path->GetEntryLabel()); - __ Bind(slow_path->GetExitLabel()); - } + // TODO: Re-add the compiler code to do string dex cache lookup again. + SlowPathCode* slow_path = new (GetGraph()->GetArena()) LoadStringSlowPathX86(load); + codegen_->AddSlowPath(slow_path); + __ jmp(slow_path->GetEntryLabel()); + __ Bind(slow_path->GetExitLabel()); } static Address GetExceptionTlsAddress() { diff --git a/compiler/optimizing/code_generator_x86_64.cc b/compiler/optimizing/code_generator_x86_64.cc index cf01a791e..9ecd14ec5 100644 --- a/compiler/optimizing/code_generator_x86_64.cc +++ b/compiler/optimizing/code_generator_x86_64.cc @@ -5635,53 +5635,15 @@ void InstructionCodeGeneratorX86_64::VisitLoadString(HLoadString* load) { codegen_->RecordSimplePatch(); return; // No dex cache slow path. } - case HLoadString::LoadKind::kDexCacheAddress: { - DCHECK_NE(load->GetAddress(), 0u); - // /* GcRoot */ out = *address - if (IsUint<32>(load->GetAddress())) { - Address address = Address::Absolute(load->GetAddress(), /* no_rip */ true); - GenerateGcRootFieldLoad(load, out_loc, address); - } else { - // TODO: Consider using opcode A1, i.e. movl eax, moff32 (with 64-bit address). - __ movq(out, Immediate(load->GetAddress())); - GenerateGcRootFieldLoad(load, out_loc, Address(out, 0)); - } - break; - } - case HLoadString::LoadKind::kDexCachePcRelative: { - uint32_t offset = load->GetDexCacheElementOffset(); - Label* fixup_label = codegen_->NewPcRelativeDexCacheArrayPatch(load->GetDexFile(), offset); - Address address = Address::Absolute(CodeGeneratorX86_64::kDummy32BitOffset, - /* no_rip */ false); - // /* GcRoot */ out = *address /* PC-relative */ - GenerateGcRootFieldLoad(load, out_loc, address, fixup_label); - break; - } - case HLoadString::LoadKind::kDexCacheViaMethod: { - CpuRegister current_method = locations->InAt(0).AsRegister(); - - // /* GcRoot */ out = current_method->declaring_class_ - GenerateGcRootFieldLoad( - load, out_loc, Address(current_method, ArtMethod::DeclaringClassOffset().Int32Value())); - // /* GcRoot[] */ out = out->dex_cache_strings_ - __ movq(out, Address(out, mirror::Class::DexCacheStringsOffset().Uint32Value())); - // /* GcRoot */ out = out[string_index] - GenerateGcRootFieldLoad( - load, out_loc, Address(out, CodeGenerator::GetCacheOffset(load->GetStringIndex()))); - break; - } default: - LOG(FATAL) << "Unexpected load kind: " << load->GetLoadKind(); - UNREACHABLE(); + break; } - if (!load->IsInDexCache()) { - SlowPathCode* slow_path = new (GetGraph()->GetArena()) LoadStringSlowPathX86_64(load); - codegen_->AddSlowPath(slow_path); - __ testl(out, out); - __ j(kEqual, slow_path->GetEntryLabel()); - __ Bind(slow_path->GetExitLabel()); - } + // TODO: Re-add the compiler code to do string dex cache lookup again. + SlowPathCode* slow_path = new (GetGraph()->GetArena()) LoadStringSlowPathX86_64(load); + codegen_->AddSlowPath(slow_path); + __ jmp(slow_path->GetEntryLabel()); + __ Bind(slow_path->GetExitLabel()); } static Address GetExceptionTlsAddress() { diff --git a/compiler/optimizing/sharpening.cc b/compiler/optimizing/sharpening.cc index b73f73893..2bea41559 100644 --- a/compiler/optimizing/sharpening.cc +++ b/compiler/optimizing/sharpening.cc @@ -279,8 +279,7 @@ void HSharpening::ProcessLoadString(HLoadString* load_string) { const DexFile& dex_file = load_string->GetDexFile(); uint32_t string_index = load_string->GetStringIndex(); - bool is_in_dex_cache = false; - HLoadString::LoadKind desired_load_kind; + HLoadString::LoadKind desired_load_kind = HLoadString::LoadKind::kDexCacheViaMethod; uint64_t address = 0u; // String or dex cache element address. { Runtime* runtime = Runtime::Current(); @@ -296,33 +295,14 @@ void HSharpening::ProcessLoadString(HLoadString* load_string) { DCHECK(!runtime->UseJitCompilation()); mirror::String* string = class_linker->ResolveString(dex_file, string_index, dex_cache); CHECK(string != nullptr); - if (!compiler_driver_->GetSupportBootImageFixup()) { - // MIPS/MIPS64 or compiler_driver_test. Do not sharpen. - desired_load_kind = HLoadString::LoadKind::kDexCacheViaMethod; - } else { - DCHECK(ContainsElement(compiler_driver_->GetDexFilesForOatFile(), &dex_file)); - is_in_dex_cache = true; - desired_load_kind = codegen_->GetCompilerOptions().GetCompilePic() - ? HLoadString::LoadKind::kBootImageLinkTimePcRelative - : HLoadString::LoadKind::kBootImageLinkTimeAddress; - } + // TODO: In follow up CL, add PcRelative and Address back in. } else if (runtime->UseJitCompilation()) { // TODO: Make sure we don't set the "compile PIC" flag for JIT as that's bogus. - // DCHECK(!codegen_->GetCompilerOptions().GetCompilePic()); + DCHECK(!codegen_->GetCompilerOptions().GetCompilePic()); mirror::String* string = dex_cache->GetResolvedString(string_index); - is_in_dex_cache = (string != nullptr); if (string != nullptr && runtime->GetHeap()->ObjectIsInBootImageSpace(string)) { - // TODO: Use direct pointers for all non-moving spaces, not just boot image. Bug: 29530787 desired_load_kind = HLoadString::LoadKind::kBootImageAddress; address = reinterpret_cast64(string); - } else { - // Note: If the string is not in the dex cache, the instruction needs environment - // and will not be inlined across dex files. Within a dex file, the slow-path helper - // loads the correct string and inlined frames are used correctly for OOM stack trace. - // TODO: Write a test for this. Bug: 29416588 - desired_load_kind = HLoadString::LoadKind::kDexCacheAddress; - void* dex_cache_element_address = &dex_cache->GetStrings()[string_index]; - address = reinterpret_cast64(dex_cache_element_address); } } else { // AOT app compilation. Try to lookup the string without allocating if not found. @@ -332,19 +312,9 @@ void HSharpening::ProcessLoadString(HLoadString* load_string) { !codegen_->GetCompilerOptions().GetCompilePic()) { desired_load_kind = HLoadString::LoadKind::kBootImageAddress; address = reinterpret_cast64(string); - } else { - // Not JIT and either the string is not in boot image or we are compiling in PIC mode. - // Use PC-relative load from the dex cache if the dex file belongs - // to the oat file that we're currently compiling. - desired_load_kind = ContainsElement(compiler_driver_->GetDexFilesForOatFile(), &dex_file) - ? HLoadString::LoadKind::kDexCachePcRelative - : HLoadString::LoadKind::kDexCacheViaMethod; } } } - if (is_in_dex_cache) { - load_string->MarkInDexCache(); - } HLoadString::LoadKind load_kind = codegen_->GetSupportedLoadStringKind(desired_load_kind); switch (load_kind) { diff --git a/patchoat/patchoat.cc b/patchoat/patchoat.cc index 943238456..3f6531b5b 100644 --- a/patchoat/patchoat.cc +++ b/patchoat/patchoat.cc @@ -37,6 +37,7 @@ #include "gc/space/image_space.h" #include "image-inl.h" #include "mirror/abstract_method.h" +#include "mirror/dex_cache.h" #include "mirror/object-inl.h" #include "mirror/method.h" #include "mirror/reference.h" @@ -592,8 +593,8 @@ void PatchOat::PatchDexFileArrays(mirror::ObjectArray* img_roots // 64-bit values here, clearing the top 32 bits for 32-bit targets. The zero-extension is // done by casting to the unsigned type uintptr_t before casting to int64_t, i.e. // static_cast(reinterpret_cast(image_begin_ + offset))). - GcRoot* orig_strings = orig_dex_cache->GetStrings(); - GcRoot* relocated_strings = RelocatedAddressOfPointer(orig_strings); + mirror::StringDexCacheType* orig_strings = orig_dex_cache->GetStrings(); + mirror::StringDexCacheType* relocated_strings = RelocatedAddressOfPointer(orig_strings); copy_dex_cache->SetField64( mirror::DexCache::StringsOffset(), static_cast(reinterpret_cast(relocated_strings))); diff --git a/runtime/class_linker-inl.h b/runtime/class_linker-inl.h index f2575f702..97aa499b2 100644 --- a/runtime/class_linker-inl.h +++ b/runtime/class_linker-inl.h @@ -27,6 +27,8 @@ #include "mirror/object_array.h" #include "handle_scope-inl.h" +#include + namespace art { inline mirror::Class* ClassLinker::FindSystemClass(Thread* self, const char* descriptor) { @@ -63,18 +65,21 @@ inline mirror::Class* ClassLinker::FindArrayClass(Thread* self, mirror::Class** inline mirror::String* ClassLinker::ResolveString(uint32_t string_idx, ArtMethod* referrer) { mirror::Class* declaring_class = referrer->GetDeclaringClass(); // MethodVerifier refuses methods with string_idx out of bounds. - DCHECK_LT(string_idx, declaring_class->GetDexCache()->NumStrings()); - mirror::String* resolved_string = declaring_class->GetDexCacheStrings()[string_idx].Read(); - if (UNLIKELY(resolved_string == nullptr)) { + DCHECK_LT(string_idx, declaring_class->GetDexFile().NumStringIds());; + mirror::String* string = + mirror::StringDexCachePair::LookupString(declaring_class->GetDexCacheStrings(), + string_idx, + mirror::DexCache::kDexCacheStringCacheSize).Read(); + if (UNLIKELY(string == nullptr)) { StackHandleScope<1> hs(Thread::Current()); Handle dex_cache(hs.NewHandle(declaring_class->GetDexCache())); const DexFile& dex_file = *dex_cache->GetDexFile(); - resolved_string = ResolveString(dex_file, string_idx, dex_cache); - if (resolved_string != nullptr) { - DCHECK_EQ(dex_cache->GetResolvedString(string_idx), resolved_string); + string = ResolveString(dex_file, string_idx, dex_cache); + if (string != nullptr) { + DCHECK_EQ(dex_cache->GetResolvedString(string_idx), string); } } - return resolved_string; + return string; } inline mirror::Class* ClassLinker::ResolveType(uint16_t type_idx, ArtMethod* referrer) { diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index 4d48da6a8..9b0fb7d6c 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -66,6 +66,7 @@ #include "mirror/class.h" #include "mirror/class-inl.h" #include "mirror/class_loader.h" +#include "mirror/dex_cache.h" #include "mirror/dex_cache-inl.h" #include "mirror/field.h" #include "mirror/iftable-inl.h" @@ -1271,7 +1272,10 @@ bool ClassLinker::UpdateAppImageClassLoadersAndDexCaches( // If the oat file expects the dex cache arrays to be in the BSS, then allocate there and // copy over the arrays. DCHECK(dex_file != nullptr); - const size_t num_strings = dex_file->NumStringIds(); + size_t num_strings = mirror::DexCache::kDexCacheStringCacheSize; + if (dex_file->NumStringIds() < num_strings) { + num_strings = dex_file->NumStringIds(); + } const size_t num_types = dex_file->NumTypeIds(); const size_t num_methods = dex_file->NumMethodIds(); const size_t num_fields = dex_file->NumFieldIds(); @@ -1281,16 +1285,17 @@ bool ClassLinker::UpdateAppImageClassLoadersAndDexCaches( CHECK_EQ(num_fields, dex_cache->NumResolvedFields()); DexCacheArraysLayout layout(image_pointer_size_, dex_file); uint8_t* const raw_arrays = oat_dex_file->GetDexCacheArrays(); - // The space is not yet visible to the GC, we can avoid the read barriers and use - // std::copy_n. if (num_strings != 0u) { - GcRoot* const image_resolved_strings = dex_cache->GetStrings(); - GcRoot* const strings = - reinterpret_cast*>(raw_arrays + layout.StringsOffset()); - for (size_t j = 0; kIsDebugBuild && j < num_strings; ++j) { - DCHECK(strings[j].IsNull()); + mirror::StringDexCacheType* const image_resolved_strings = dex_cache->GetStrings(); + mirror::StringDexCacheType* const strings = + reinterpret_cast(raw_arrays + layout.StringsOffset()); + for (size_t j = 0; j < num_strings; ++j) { + DCHECK_EQ(strings[j].load(std::memory_order_relaxed).string_index, 0u); + DCHECK(strings[j].load(std::memory_order_relaxed).string_pointer.IsNull()); + strings[j].store(image_resolved_strings[j].load(std::memory_order_relaxed), + std::memory_order_relaxed); } - std::copy_n(image_resolved_strings, num_strings, strings); + mirror::StringDexCachePair::Initialize(strings); dex_cache->SetStrings(strings); } if (num_types != 0u) { @@ -1473,14 +1478,14 @@ class UpdateClassLoaderAndResolvedStringsVisitor { bool operator()(mirror::Class* klass) const SHARED_REQUIRES(Locks::mutator_lock_) { if (forward_strings_) { - GcRoot* strings = klass->GetDexCacheStrings(); + mirror::StringDexCacheType* strings = klass->GetDexCacheStrings(); if (strings != nullptr) { DCHECK( space_->GetImageHeader().GetImageSection(ImageHeader::kSectionDexCacheArrays).Contains( reinterpret_cast(strings) - space_->Begin())) << "String dex cache array for " << PrettyClass(klass) << " is not in app image"; // Dex caches have already been updated, so take the strings pointer from there. - GcRoot* new_strings = klass->GetDexCache()->GetStrings(); + mirror::StringDexCacheType* new_strings = klass->GetDexCache()->GetStrings(); DCHECK_NE(strings, new_strings); klass->SetDexCacheStrings(new_strings); } @@ -2079,18 +2084,27 @@ mirror::DexCache* ClassLinker::AllocDexCache(Thread* self, // Zero-initialized. raw_arrays = reinterpret_cast(linear_alloc->Alloc(self, layout.Size())); } - GcRoot* strings = (dex_file.NumStringIds() == 0u) ? nullptr : - reinterpret_cast*>(raw_arrays + layout.StringsOffset()); + mirror::StringDexCacheType* strings = (dex_file.NumStringIds() == 0u) ? nullptr : + reinterpret_cast(raw_arrays + layout.StringsOffset()); GcRoot* types = (dex_file.NumTypeIds() == 0u) ? nullptr : reinterpret_cast*>(raw_arrays + layout.TypesOffset()); ArtMethod** methods = (dex_file.NumMethodIds() == 0u) ? nullptr : reinterpret_cast(raw_arrays + layout.MethodsOffset()); ArtField** fields = (dex_file.NumFieldIds() == 0u) ? nullptr : reinterpret_cast(raw_arrays + layout.FieldsOffset()); + size_t num_strings = mirror::DexCache::kDexCacheStringCacheSize; + if (dex_file.NumStringIds() < num_strings) { + num_strings = dex_file.NumStringIds(); + } + DCHECK_ALIGNED(strings, alignof(mirror::StringDexCacheType)) << + "Expected strings to align to StringDexCacheType."; + static_assert(alignof(mirror::StringDexCacheType) == 8u, + "Expected StringDexCacheType to have align of 8."); if (kIsDebugBuild) { // Sanity check to make sure all the dex cache arrays are empty. b/28992179 - for (size_t i = 0; i < dex_file.NumStringIds(); ++i) { - CHECK(strings[i].Read() == nullptr); + for (size_t i = 0; i < num_strings; ++i) { + CHECK_EQ(strings[i].load(std::memory_order_relaxed).string_index, 0u); + CHECK(strings[i].load(std::memory_order_relaxed).string_pointer.IsNull()); } for (size_t i = 0; i < dex_file.NumTypeIds(); ++i) { CHECK(types[i].Read() == nullptr); @@ -2102,10 +2116,13 @@ mirror::DexCache* ClassLinker::AllocDexCache(Thread* self, CHECK(mirror::DexCache::GetElementPtrSize(fields, i, image_pointer_size_) == nullptr); } } + if (strings != nullptr) { + mirror::StringDexCachePair::Initialize(strings); + } dex_cache->Init(&dex_file, location.Get(), strings, - dex_file.NumStringIds(), + num_strings, types, dex_file.NumTypeIds(), methods, diff --git a/runtime/gc/space/image_space.cc b/runtime/gc/space/image_space.cc index 4505c249f..ae6c3214a 100644 --- a/runtime/gc/space/image_space.cc +++ b/runtime/gc/space/image_space.cc @@ -1197,9 +1197,9 @@ class ImageSpaceLoader { for (int32_t i = 0, count = dex_caches->GetLength(); i < count; ++i) { mirror::DexCache* dex_cache = dex_caches->Get(i); // Fix up dex cache pointers. - GcRoot* strings = dex_cache->GetStrings(); + mirror::StringDexCacheType* strings = dex_cache->GetStrings(); if (strings != nullptr) { - GcRoot* new_strings = fixup_adapter.ForwardObject(strings); + mirror::StringDexCacheType* new_strings = fixup_adapter.ForwardObject(strings); if (strings != new_strings) { dex_cache->SetStrings(new_strings); } diff --git a/runtime/interpreter/interpreter_common.h b/runtime/interpreter/interpreter_common.h index 4fd1514e3..90c822744 100644 --- a/runtime/interpreter/interpreter_common.h +++ b/runtime/interpreter/interpreter_common.h @@ -23,6 +23,7 @@ #include #include +#include #include "art_field-inl.h" #include "art_method-inl.h" @@ -37,6 +38,8 @@ #include "handle_scope-inl.h" #include "jit/jit.h" #include "mirror/class-inl.h" +#include "mirror/dex_cache.h" +#include "mirror/method.h" #include "mirror/object-inl.h" #include "mirror/object_array-inl.h" #include "mirror/string-inl.h" @@ -264,15 +267,20 @@ static inline String* ResolveString(Thread* self, ShadowFrame& shadow_frame, uin ArtMethod* method = shadow_frame.GetMethod(); mirror::Class* declaring_class = method->GetDeclaringClass(); // MethodVerifier refuses methods with string_idx out of bounds. - DCHECK_LT(string_idx, declaring_class->GetDexCache()->NumStrings()); - mirror::String* s = declaring_class->GetDexCacheStrings()[string_idx].Read(); - if (UNLIKELY(s == nullptr)) { + DCHECK_LT(string_idx % mirror::DexCache::kDexCacheStringCacheSize, + declaring_class->GetDexFile().NumStringIds()); + mirror::String* string_ptr = + mirror::StringDexCachePair::LookupString(declaring_class->GetDexCacheStrings(), + string_idx, + mirror::DexCache::kDexCacheStringCacheSize).Read(); + if (UNLIKELY(string_ptr == nullptr)) { StackHandleScope<1> hs(self); Handle dex_cache(hs.NewHandle(declaring_class->GetDexCache())); - s = Runtime::Current()->GetClassLinker()->ResolveString(*method->GetDexFile(), string_idx, - dex_cache); + string_ptr = Runtime::Current()->GetClassLinker()->ResolveString(*method->GetDexFile(), + string_idx, + dex_cache); } - return s; + return string_ptr; } // Handles div-int, div-int/2addr, div-int/li16 and div-int/lit8 instructions. diff --git a/runtime/mirror/class-inl.h b/runtime/mirror/class-inl.h index 8ad47eb79..0f2aac279 100644 --- a/runtime/mirror/class-inl.h +++ b/runtime/mirror/class-inl.h @@ -26,7 +26,6 @@ #include "base/length_prefixed_array.h" #include "class_loader.h" #include "common_throws.h" -#include "dex_cache.h" #include "dex_file.h" #include "gc/heap-inl.h" #include "iftable.h" @@ -899,12 +898,12 @@ inline uint32_t Class::NumDirectInterfaces() { } } -inline void Class::SetDexCacheStrings(GcRoot* new_dex_cache_strings) { +inline void Class::SetDexCacheStrings(StringDexCacheType* new_dex_cache_strings) { SetFieldPtr(DexCacheStringsOffset(), new_dex_cache_strings); } -inline GcRoot* Class::GetDexCacheStrings() { - return GetFieldPtr*>(DexCacheStringsOffset()); +inline StringDexCacheType* Class::GetDexCacheStrings() { + return GetFieldPtr64(DexCacheStringsOffset()); } template @@ -1058,8 +1057,8 @@ inline void Class::FixupNativePointers(mirror::Class* dest, dest->SetMethodsPtrInternal(new_methods); } // Update dex cache strings. - GcRoot* strings = GetDexCacheStrings(); - GcRoot* new_strings = visitor(strings); + StringDexCacheType* strings = GetDexCacheStrings(); + StringDexCacheType* new_strings = visitor(strings); if (strings != new_strings) { dest->SetDexCacheStrings(new_strings); } diff --git a/runtime/mirror/class.h b/runtime/mirror/class.h index 978fc4cbb..e2cd649d9 100644 --- a/runtime/mirror/class.h +++ b/runtime/mirror/class.h @@ -54,6 +54,9 @@ class Constructor; class DexCache; class IfTable; class Method; +struct StringDexCachePair; + +using StringDexCacheType = std::atomic; // C++ mirror of java.lang.Class class MANAGED Class FINAL : public Object { @@ -1219,8 +1222,8 @@ class MANAGED Class FINAL : public Object { bool GetSlowPathEnabled() SHARED_REQUIRES(Locks::mutator_lock_); void SetSlowPath(bool enabled) SHARED_REQUIRES(Locks::mutator_lock_); - GcRoot* GetDexCacheStrings() SHARED_REQUIRES(Locks::mutator_lock_); - void SetDexCacheStrings(GcRoot* new_dex_cache_strings) + StringDexCacheType* GetDexCacheStrings() SHARED_REQUIRES(Locks::mutator_lock_); + void SetDexCacheStrings(StringDexCacheType* new_dex_cache_strings) SHARED_REQUIRES(Locks::mutator_lock_); static MemberOffset DexCacheStringsOffset() { return OFFSET_OF_OBJECT_MEMBER(Class, dex_cache_strings_); diff --git a/runtime/mirror/dex_cache-inl.h b/runtime/mirror/dex_cache-inl.h index 84469ea86..a3071b7f6 100644 --- a/runtime/mirror/dex_cache-inl.h +++ b/runtime/mirror/dex_cache-inl.h @@ -27,6 +27,8 @@ #include "mirror/class.h" #include "runtime.h" +#include + namespace art { namespace mirror { @@ -35,15 +37,18 @@ inline uint32_t DexCache::ClassSize(PointerSize pointer_size) { return Class::ComputeClassSize(true, vtable_entries, 0, 0, 0, 0, 0, pointer_size); } -inline String* DexCache::GetResolvedString(uint32_t string_idx) { - DCHECK_LT(string_idx, NumStrings()); - return GetStrings()[string_idx].Read(); +inline mirror::String* DexCache::GetResolvedString(uint32_t string_idx) { + DCHECK_LT(string_idx, GetDexFile()->NumStringIds()); + return StringDexCachePair::LookupString(GetStrings(), string_idx, NumStrings()).Read(); } -inline void DexCache::SetResolvedString(uint32_t string_idx, String* resolved) { - DCHECK_LT(string_idx, NumStrings()); +inline void DexCache::SetResolvedString(uint32_t string_idx, mirror::String* resolved) { + DCHECK_LT(string_idx % NumStrings(), NumStrings()); // TODO default transaction support. - GetStrings()[string_idx] = GcRoot(resolved); + StringDexCachePair idx_ptr; + idx_ptr.string_index = string_idx; + idx_ptr.string_pointer = GcRoot(resolved); + GetStrings()[string_idx % NumStrings()].store(idx_ptr, std::memory_order_relaxed); // TODO: Fine-grained marking, so that we don't need to go through all arrays in full. Runtime::Current()->GetHeap()->WriteBarrierEveryFieldOf(this); } @@ -131,9 +136,16 @@ inline void DexCache::VisitReferences(mirror::Class* klass, const Visitor& visit VisitInstanceFieldsReferences(klass, visitor); // Visit arrays after. if (kVisitNativeRoots) { - GcRoot* strings = GetStrings(); + mirror::StringDexCacheType* strings = GetStrings(); for (size_t i = 0, num_strings = NumStrings(); i != num_strings; ++i) { - visitor.VisitRootIfNonNull(strings[i].AddressWithoutBarrier()); + StringDexCachePair source = strings[i].load(std::memory_order_relaxed); + mirror::String* before = source.string_pointer.Read(); + GcRoot root(before); + visitor.VisitRootIfNonNull(root.AddressWithoutBarrier()); + if (root.Read() != before) { + source.string_pointer = GcRoot(root.Read()); + strings[i].store(source, std::memory_order_relaxed); + } } GcRoot* resolved_types = GetResolvedTypes(); for (size_t i = 0, num_types = NumResolvedTypes(); i != num_types; ++i) { @@ -143,12 +155,14 @@ inline void DexCache::VisitReferences(mirror::Class* klass, const Visitor& visit } template -inline void DexCache::FixupStrings(GcRoot* dest, const Visitor& visitor) { - GcRoot* src = GetStrings(); +inline void DexCache::FixupStrings(mirror::StringDexCacheType* dest, const Visitor& visitor) { + mirror::StringDexCacheType* src = GetStrings(); for (size_t i = 0, count = NumStrings(); i < count; ++i) { - mirror::String* source = src[i].Read(); - mirror::String* new_source = visitor(source); - dest[i] = GcRoot(new_source); + StringDexCachePair source = src[i].load(std::memory_order_relaxed); + mirror::String* ptr = source.string_pointer.Read(); + mirror::String* new_source = visitor(ptr); + source.string_pointer = GcRoot(new_source); + dest[i].store(source, std::memory_order_relaxed); } } diff --git a/runtime/mirror/dex_cache.cc b/runtime/mirror/dex_cache.cc index 57066d837..cfcec9cd3 100644 --- a/runtime/mirror/dex_cache.cc +++ b/runtime/mirror/dex_cache.cc @@ -33,7 +33,7 @@ namespace mirror { void DexCache::Init(const DexFile* dex_file, String* location, - GcRoot* strings, + StringDexCacheType* strings, uint32_t num_strings, GcRoot* resolved_types, uint32_t num_resolved_types, diff --git a/runtime/mirror/dex_cache.h b/runtime/mirror/dex_cache.h index d02a0d8e2..e04bde04b 100644 --- a/runtime/mirror/dex_cache.h +++ b/runtime/mirror/dex_cache.h @@ -35,12 +35,62 @@ namespace mirror { class String; +struct StringDexCachePair { + GcRoot string_pointer; + uint32_t string_index; + // The array is initially [ {0,0}, {0,0}, {0,0} ... ] + // We maintain the invariant that once a dex cache entry is populated, + // the pointer is always non-0 + // Any given entry would thus be: + // {non-0, non-0} OR {0,0} + // + // It's generally sufficiently enough then to check if the + // lookup string index matches the stored string index (for a >0 string index) + // because if it's true the pointer is also non-null. + // + // For the 0th entry which is a special case, the value is either + // {0,0} (initial state) or {non-0, 0} which indicates + // that a valid string is stored at that index for a dex string id of 0. + // + // As an optimization, we want to avoid branching on the string pointer since + // it's always non-null if the string id branch succeeds (except for the 0th string id). + // Set the initial state for the 0th entry to be {0,1} which is guaranteed to fail + // the lookup string id == stored id branch. + static void Initialize(StringDexCacheType* strings) { + DCHECK(StringDexCacheType().is_lock_free()); + mirror::StringDexCachePair first_elem; + first_elem.string_pointer = GcRoot(nullptr); + first_elem.string_index = 1; + strings[0].store(first_elem, std::memory_order_relaxed); + } + static GcRoot LookupString(StringDexCacheType* dex_cache, + uint32_t string_idx, + uint32_t cache_size) { + StringDexCachePair index_string = dex_cache[string_idx % cache_size] + .load(std::memory_order_relaxed); + if (string_idx != index_string.string_index) return GcRoot(nullptr); + DCHECK(!index_string.string_pointer.IsNull()); + return index_string.string_pointer; + } +}; +using StringDexCacheType = std::atomic; + + // C++ mirror of java.lang.DexCache. class MANAGED DexCache FINAL : public Object { public: // Size of java.lang.DexCache.class. static uint32_t ClassSize(PointerSize pointer_size); + // Size of string dex cache. Needs to be a power of 2 for entrypoint assumptions to hold. + static constexpr size_t kDexCacheStringCacheSize = 1024; + static_assert(IsPowerOfTwo(kDexCacheStringCacheSize), + "String dex cache size is not a power of 2."); + + static constexpr size_t StaticStringSize() { + return kDexCacheStringCacheSize; + } + // Size of an instance of java.lang.DexCache not including referenced values. static constexpr uint32_t InstanceSize() { return sizeof(DexCache); @@ -48,7 +98,7 @@ class MANAGED DexCache FINAL : public Object { void Init(const DexFile* dex_file, String* location, - GcRoot* strings, + StringDexCacheType* strings, uint32_t num_strings, GcRoot* resolved_types, uint32_t num_resolved_types, @@ -62,7 +112,7 @@ class MANAGED DexCache FINAL : public Object { SHARED_REQUIRES(Locks::mutator_lock_); template - void FixupStrings(GcRoot* dest, const Visitor& visitor) + void FixupStrings(StringDexCacheType* dest, const Visitor& visitor) SHARED_REQUIRES(Locks::mutator_lock_); template @@ -109,10 +159,10 @@ class MANAGED DexCache FINAL : public Object { return OFFSET_OF_OBJECT_MEMBER(DexCache, num_resolved_methods_); } - String* GetResolvedString(uint32_t string_idx) ALWAYS_INLINE + mirror::String* GetResolvedString(uint32_t string_idx) ALWAYS_INLINE SHARED_REQUIRES(Locks::mutator_lock_); - void SetResolvedString(uint32_t string_idx, String* resolved) ALWAYS_INLINE + void SetResolvedString(uint32_t string_idx, mirror::String* resolved) ALWAYS_INLINE SHARED_REQUIRES(Locks::mutator_lock_); Class* GetResolvedType(uint32_t type_idx) SHARED_REQUIRES(Locks::mutator_lock_); @@ -135,11 +185,11 @@ class MANAGED DexCache FINAL : public Object { ALWAYS_INLINE void SetResolvedField(uint32_t idx, ArtField* field, PointerSize ptr_size) SHARED_REQUIRES(Locks::mutator_lock_); - GcRoot* GetStrings() ALWAYS_INLINE SHARED_REQUIRES(Locks::mutator_lock_) { - return GetFieldPtr*>(StringsOffset()); + StringDexCacheType* GetStrings() ALWAYS_INLINE SHARED_REQUIRES(Locks::mutator_lock_) { + return GetFieldPtr64(StringsOffset()); } - void SetStrings(GcRoot* strings) ALWAYS_INLINE SHARED_REQUIRES(Locks::mutator_lock_) { + void SetStrings(StringDexCacheType* strings) ALWAYS_INLINE SHARED_REQUIRES(Locks::mutator_lock_) { SetFieldPtr(StringsOffset(), strings); } @@ -224,7 +274,8 @@ class MANAGED DexCache FINAL : public Object { uint64_t resolved_fields_; // ArtField*, array with num_resolved_fields_ elements. uint64_t resolved_methods_; // ArtMethod*, array with num_resolved_methods_ elements. uint64_t resolved_types_; // GcRoot*, array with num_resolved_types_ elements. - uint64_t strings_; // GcRoot*, array with num_strings_ elements. + uint64_t strings_; // std::atomic*, + // array with num_strings_ elements. uint32_t num_resolved_fields_; // Number of elements in the resolved_fields_ array. uint32_t num_resolved_methods_; // Number of elements in the resolved_methods_ array. uint32_t num_resolved_types_; // Number of elements in the resolved_types_ array. diff --git a/runtime/mirror/dex_cache_test.cc b/runtime/mirror/dex_cache_test.cc index 48f2ca59e..175997c2d 100644 --- a/runtime/mirror/dex_cache_test.cc +++ b/runtime/mirror/dex_cache_test.cc @@ -22,6 +22,7 @@ #include "common_runtime_test.h" #include "linear_alloc.h" #include "mirror/class_loader-inl.h" +#include "mirror/dex_cache-inl.h" #include "handle_scope-inl.h" #include "scoped_thread_state_change.h" @@ -40,7 +41,8 @@ TEST_F(DexCacheTest, Open) { Runtime::Current()->GetLinearAlloc()))); ASSERT_TRUE(dex_cache.Get() != nullptr); - EXPECT_EQ(java_lang_dex_file_->NumStringIds(), dex_cache->NumStrings()); + EXPECT_TRUE(dex_cache->StaticStringSize() == dex_cache->NumStrings() + || java_lang_dex_file_->NumStringIds() == dex_cache->NumStrings()); EXPECT_EQ(java_lang_dex_file_->NumTypeIds(), dex_cache->NumResolvedTypes()); EXPECT_EQ(java_lang_dex_file_->NumMethodIds(), dex_cache->NumResolvedMethods()); EXPECT_EQ(java_lang_dex_file_->NumFieldIds(), dex_cache->NumResolvedFields()); diff --git a/runtime/native/java_lang_DexCache.cc b/runtime/native/java_lang_DexCache.cc index 994ccb1ad..f0140a303 100644 --- a/runtime/native/java_lang_DexCache.cc +++ b/runtime/native/java_lang_DexCache.cc @@ -59,7 +59,7 @@ static jobject DexCache_getResolvedType(JNIEnv* env, jobject javaDexCache, jint static jobject DexCache_getResolvedString(JNIEnv* env, jobject javaDexCache, jint string_index) { ScopedFastNativeObjectAccess soa(env); mirror::DexCache* dex_cache = soa.Decode(javaDexCache); - CHECK_LT(static_cast(string_index), dex_cache->NumStrings()); + CHECK_LT(static_cast(string_index), dex_cache->GetDexFile()->NumStringIds()); return soa.AddLocalReference(dex_cache->GetResolvedString(string_index)); } @@ -75,7 +75,7 @@ static void DexCache_setResolvedString(JNIEnv* env, jobject javaDexCache, jint s jobject string) { ScopedFastNativeObjectAccess soa(env); mirror::DexCache* dex_cache = soa.Decode(javaDexCache); - CHECK_LT(static_cast(string_index), dex_cache->NumStrings()); + CHECK_LT(static_cast(string_index), dex_cache->GetDexFile()->NumStringIds()); dex_cache->SetResolvedString(string_index, soa.Decode(string)); } diff --git a/runtime/utils/dex_cache_arrays_layout-inl.h b/runtime/utils/dex_cache_arrays_layout-inl.h index 7733a51aa..4752a8652 100644 --- a/runtime/utils/dex_cache_arrays_layout-inl.h +++ b/runtime/utils/dex_cache_arrays_layout-inl.h @@ -23,6 +23,7 @@ #include "base/logging.h" #include "gc_root.h" #include "globals.h" +#include "mirror/dex_cache.h" #include "primitive.h" namespace art { @@ -45,12 +46,11 @@ inline DexCacheArraysLayout::DexCacheArraysLayout(PointerSize pointer_size, cons : DexCacheArraysLayout(pointer_size, dex_file->GetHeader()) { } -inline size_t DexCacheArraysLayout::Alignment() const { +inline constexpr size_t DexCacheArraysLayout::Alignment() { // GcRoot<> alignment is 4, i.e. lower than or equal to the pointer alignment. static_assert(alignof(GcRoot) == 4, "Expecting alignof(GcRoot<>) == 4"); - static_assert(alignof(GcRoot) == 4, "Expecting alignof(GcRoot<>) == 4"); - // Pointer alignment is the same as pointer size. - return static_cast(pointer_size_); + static_assert(alignof(mirror::StringDexCacheType) == 8, "Expecting alignof(StringDexCacheType) == 8"); + return sizeof(mirror::StringDexCacheType); } template @@ -87,15 +87,20 @@ inline size_t DexCacheArraysLayout::MethodsAlignment() const { } inline size_t DexCacheArraysLayout::StringOffset(uint32_t string_idx) const { - return strings_offset_ + ElementOffset(GcRootAsPointerSize(), string_idx); + return strings_offset_ + ElementOffset(PointerSize::k64, + string_idx % mirror::DexCache::kDexCacheStringCacheSize); } inline size_t DexCacheArraysLayout::StringsSize(size_t num_elements) const { - return ArraySize(GcRootAsPointerSize(), num_elements); + size_t cache_size = mirror::DexCache::kDexCacheStringCacheSize; + if (num_elements < cache_size) { + cache_size = num_elements; + } + return ArraySize(PointerSize::k64, cache_size); } inline size_t DexCacheArraysLayout::StringsAlignment() const { - return alignof(GcRoot); + return alignof(uint64_t); } inline size_t DexCacheArraysLayout::FieldOffset(uint32_t field_idx) const { diff --git a/runtime/utils/dex_cache_arrays_layout.h b/runtime/utils/dex_cache_arrays_layout.h index f2437fa55..20ffa9059 100644 --- a/runtime/utils/dex_cache_arrays_layout.h +++ b/runtime/utils/dex_cache_arrays_layout.h @@ -52,7 +52,7 @@ class DexCacheArraysLayout { return size_; } - size_t Alignment() const; + static constexpr size_t Alignment(); size_t TypesOffset() const { return types_offset_; diff --git a/test/Android.run-test.mk b/test/Android.run-test.mk index 65debc9a5..09f969e85 100644 --- a/test/Android.run-test.mk +++ b/test/Android.run-test.mk @@ -225,9 +225,11 @@ ART_TEST_KNOWN_BROKEN += $(call all-run-test-names,$(TARGET_TYPES),$(RUN_TYPES), # Disable 149-suspend-all-stress, its output is flaky (b/28988206). # Disable 577-profile-foreign-dex (b/27454772). +# Disable 552-checker-sharpening, until compiler component of new string dex cache is added (@cwadsworth, @vmarko) TEST_ART_BROKEN_ALL_TARGET_TESTS := \ 149-suspend-all-stress \ 577-profile-foreign-dex \ + 552-checker-sharpening \ ART_TEST_KNOWN_BROKEN += $(call all-run-test-names,$(TARGET_TYPES),$(RUN_TYPES),$(PREBUILD_TYPES), \ $(COMPILER_TYPES), $(RELOCATE_TYPES),$(TRACE_TYPES),$(GC_TYPES),$(JNI_TYPES), \ -- 2.11.0