diff options
| -rw-r--r-- | dex2oat/linker/image_writer.cc | 121 | ||||
| -rw-r--r-- | dex2oat/linker/image_writer.h | 11 | ||||
| -rw-r--r-- | test/1001-app-image-regions/app_image_regions.cc | 53 | ||||
| -rwxr-xr-x | test/1001-app-image-regions/build | 34 | ||||
| -rw-r--r-- | test/1001-app-image-regions/expected.txt | 4 | ||||
| -rw-r--r-- | test/1001-app-image-regions/info.txt | 1 | ||||
| -rw-r--r-- | test/1001-app-image-regions/run | 17 | ||||
| -rw-r--r-- | test/1001-app-image-regions/src/Main.java | 37 | ||||
| -rw-r--r-- | test/Android.bp | 1 | ||||
| -rw-r--r-- | test/knownfailures.json | 7 |
10 files changed, 258 insertions, 28 deletions
diff --git a/dex2oat/linker/image_writer.cc b/dex2oat/linker/image_writer.cc index 1331fc3bac..7733cb7544 100644 --- a/dex2oat/linker/image_writer.cc +++ b/dex2oat/linker/image_writer.cc @@ -49,6 +49,7 @@ #include "gc/heap-visit-objects-inl.h" #include "gc/heap.h" #include "gc/space/large_object_space.h" +#include "gc/space/region_space.h" #include "gc/space/space-inl.h" #include "gc/verification.h" #include "handle_scope-inl.h" @@ -2423,10 +2424,31 @@ void ImageWriter::CalculateNewObjectOffsets() { } // Calculate bin slot offsets. - for (ImageInfo& image_info : image_infos_) { + for (size_t oat_index = 0; oat_index < image_infos_.size(); ++oat_index) { + ImageInfo& image_info = image_infos_[oat_index]; size_t bin_offset = image_objects_offset_begin_; + // Need to visit the objects in bin order since alignment requirements might change the + // section sizes. + using BinPair = std::pair<BinSlot, ObjPtr<mirror::Object>>; + std::vector<BinPair> objects; + heap->VisitObjects([&](mirror::Object* obj) + REQUIRES_SHARED(Locks::mutator_lock_) { + // Only visit the oat index for the current image. + if (IsImageObject(obj) && GetOatIndex(obj) == oat_index) { + objects.emplace_back(GetImageBinSlot(obj), obj); + } + }); + std::sort(objects.begin(), objects.end(), [](const BinPair& a, const BinPair& b) -> bool { + if (a.first.GetBin() != b.first.GetBin()) { + return a.first.GetBin() < b.first.GetBin(); + } + // Note that the index is really the relative offset in this case. + return a.first.GetIndex() < b.first.GetIndex(); + }); + auto it = objects.begin(); for (size_t i = 0; i != kNumberOfBins; ++i) { - switch (static_cast<Bin>(i)) { + Bin bin = enum_cast<Bin>(i); + switch (bin) { case Bin::kArtMethodClean: case Bin::kArtMethodDirty: { bin_offset = RoundUp(bin_offset, method_alignment); @@ -2445,13 +2467,49 @@ void ImageWriter::CalculateNewObjectOffsets() { } } image_info.bin_slot_offsets_[i] = bin_offset; - bin_offset += image_info.bin_slot_sizes_[i]; + + // If the bin is for mirror objects, assign the offsets since we may need to change sizes + // from alignment requirements. + if (i < static_cast<size_t>(Bin::kMirrorCount)) { + const size_t start_offset = bin_offset; + // Visit and assign offsets for all objects of the bin type. + while (it != objects.end() && it->first.GetBin() == bin) { + ObjPtr<mirror::Object> obj(it->second); + const size_t object_size = RoundUp(obj->SizeOf(), kObjectAlignment); + // If the object spans region bondaries, add padding objects between. + // TODO: Instead of adding padding, we should consider reordering the bins to reduce + // wasted space. + if (region_size_ != 0u) { + const size_t offset_after_header = bin_offset - sizeof(ImageHeader); + const size_t next_region = RoundUp(offset_after_header, region_size_); + if (offset_after_header != next_region && + offset_after_header + object_size > next_region) { + // Add padding objects until aligned. + while (bin_offset - sizeof(ImageHeader) < next_region) { + image_info.padding_object_offsets_.push_back(bin_offset); + bin_offset += kObjectAlignment; + region_alignment_wasted_ += kObjectAlignment; + image_info.image_end_ += kObjectAlignment; + } + CHECK_EQ(bin_offset - sizeof(ImageHeader), next_region); + } + } + SetImageOffset(obj.Ptr(), bin_offset); + bin_offset = bin_offset + object_size; + ++it; + } + image_info.bin_slot_sizes_[i] = bin_offset - start_offset; + } else { + bin_offset += image_info.bin_slot_sizes_[i]; + } } // NOTE: There may be additional padding between the bin slots and the intern table. DCHECK_EQ(image_info.image_end_, image_info.GetBinSizeSum(Bin::kMirrorCount) + image_objects_offset_begin_); } + VLOG(image) << "Space wasted for region alignment " << region_alignment_wasted_; + // Calculate image offsets. size_t image_offset = 0; for (ImageInfo& image_info : image_infos_) { @@ -2462,17 +2520,6 @@ void ImageWriter::CalculateNewObjectOffsets() { image_offset += image_info.image_size_; } - // Transform each object's bin slot into an offset which will be used to do the final copy. - { - auto unbin_objects_into_offset = [&](mirror::Object* obj) - REQUIRES_SHARED(Locks::mutator_lock_) { - if (IsImageObject(obj)) { - UnbinObjectsIntoOffset(obj); - } - }; - heap->VisitObjects(unbin_objects_into_offset); - } - size_t i = 0; for (ImageInfo& image_info : image_infos_) { image_info.image_roots_address_ = PointerToLowMemUInt32(GetImageAddress(image_roots[i].Get())); @@ -2887,16 +2934,6 @@ void ImageWriter::CopyAndFixupNativeData(size_t oat_index) { } } -void ImageWriter::CopyAndFixupObjects() { - auto visitor = [&](Object* obj) REQUIRES_SHARED(Locks::mutator_lock_) { - DCHECK(obj != nullptr); - CopyAndFixupObject(obj); - }; - Runtime::Current()->GetHeap()->VisitObjects(visitor); - // We no longer need the hashcode map, values have already been copied to target objects. - saved_hashcode_map_.clear(); -} - void ImageWriter::FixupPointerArray(mirror::Object* dst, mirror::PointerArray* arr, Bin array_type) { @@ -2944,6 +2981,18 @@ void ImageWriter::CopyAndFixupObject(Object* obj) { image_info.image_bitmap_->Set(dst); // Mark the obj as live. const size_t n = obj->SizeOf(); + + if (kIsDebugBuild && region_size_ != 0u) { + const size_t offset_after_header = offset - sizeof(ImageHeader); + const size_t next_region = RoundUp(offset_after_header, region_size_); + if (offset_after_header != next_region) { + // If the object is not on a region bondary, it must not be cross region. + CHECK_LT(offset_after_header, next_region) + << "offset_after_header=" << offset_after_header << " size=" << n; + CHECK_LE(offset_after_header + n, next_region) + << "offset_after_header=" << offset_after_header << " size=" << n; + } + } DCHECK_LE(offset + n, image_info.image_.Size()); memcpy(dst, src, n); @@ -2973,7 +3022,6 @@ class ImageWriter::FixupVisitor { const {} void VisitRoot(mirror::CompressedReference<mirror::Object>* root ATTRIBUTE_UNUSED) const {} - void operator()(ObjPtr<Object> obj, MemberOffset offset, bool is_static ATTRIBUTE_UNUSED) const REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(Locks::heap_bitmap_lock_) { ObjPtr<Object> ref = obj->GetFieldObject<Object, kVerifyNone>(offset); @@ -2994,6 +3042,25 @@ class ImageWriter::FixupVisitor { mirror::Object* const copy_; }; +void ImageWriter::CopyAndFixupObjects() { + auto visitor = [&](Object* obj) REQUIRES_SHARED(Locks::mutator_lock_) { + DCHECK(obj != nullptr); + CopyAndFixupObject(obj); + }; + Runtime::Current()->GetHeap()->VisitObjects(visitor); + // Copy the padding objects since they are required for in order traversal of the image space. + for (const ImageInfo& image_info : image_infos_) { + for (const size_t offset : image_info.padding_object_offsets_) { + auto* dst = reinterpret_cast<Object*>(image_info.image_.Begin() + offset); + dst->SetClass<kVerifyNone>(GetImageAddress(GetClassRoot<mirror::Object>().Ptr())); + dst->SetLockWord<kVerifyNone>(LockWord::Default(), /*as_volatile=*/ false); + image_info.image_bitmap_->Set(dst); // Mark the obj as live. + } + } + // We no longer need the hashcode map, values have already been copied to target objects. + saved_hashcode_map_.clear(); +} + class ImageWriter::FixupClassVisitor final : public FixupVisitor { public: FixupClassVisitor(ImageWriter* image_writer, Object* copy) @@ -3572,6 +3639,10 @@ ImageWriter::ImageWriter( CHECK_EQ(compiler_options.IsBootImage(), Runtime::Current()->GetHeap()->GetBootImageSpaces().empty()) << "Compiling a boot image should occur iff there are no boot image spaces loaded"; + if (compiler_options_.IsAppImage()) { + // Make sure objects are not crossing region boundaries for app images. + region_size_ = gc::space::RegionSpace::kRegionSize; + } } ImageWriter::ImageInfo::ImageInfo() diff --git a/dex2oat/linker/image_writer.h b/dex2oat/linker/image_writer.h index b680265ff1..8896e07026 100644 --- a/dex2oat/linker/image_writer.h +++ b/dex2oat/linker/image_writer.h @@ -279,7 +279,7 @@ class ImageWriter final { private: // Must be the same size as LockWord, any larger and we would truncate the data. - const uint32_t lockword_; + uint32_t lockword_; }; struct ImageInfo { @@ -397,6 +397,9 @@ class ImageWriter final { // Class table associated with this image for serialization. std::unique_ptr<ClassTable> class_table_; + + // Padding objects to ensure region alignment (if required). + std::vector<size_t> padding_object_offsets_; }; // We use the lock word to store the offset of the object in the image. @@ -798,6 +801,12 @@ class ImageWriter final { // Set of objects known to be dirty in the image. Can be nullptr if there are none. const HashSet<std::string>* dirty_image_objects_; + // Objects are guaranteed to not cross the region size boundary. + size_t region_size_ = 0u; + + // Region alignment bytes wasted. + size_t region_alignment_wasted_ = 0u; + class ImageFileGuard; class FixupClassVisitor; class FixupRootVisitor; diff --git a/test/1001-app-image-regions/app_image_regions.cc b/test/1001-app-image-regions/app_image_regions.cc new file mode 100644 index 0000000000..dc16a84928 --- /dev/null +++ b/test/1001-app-image-regions/app_image_regions.cc @@ -0,0 +1,53 @@ +/* + * Copyright 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +#include "jni.h" + +#include "gc/heap.h" +#include "gc/space/image_space.h" +#include "gc/space/space-inl.h" +#include "gc/space/region_space.h" +#include "image.h" +#include "mirror/class.h" +#include "runtime.h" +#include "scoped_thread_state_change-inl.h" + +namespace art { + +namespace { + +extern "C" JNIEXPORT jint JNICALL Java_Main_getRegionSize(JNIEnv*, jclass) { + return gc::space::RegionSpace::kRegionSize; +} + +extern "C" JNIEXPORT jint JNICALL Java_Main_checkAppImageSectionSize(JNIEnv*, jclass, jclass c) { + ScopedObjectAccess soa(Thread::Current()); + ObjPtr<mirror::Class> klass_ptr = soa.Decode<mirror::Class>(c); + for (auto* space : Runtime::Current()->GetHeap()->GetContinuousSpaces()) { + if (space->IsImageSpace()) { + auto* image_space = space->AsImageSpace(); + const auto& image_header = image_space->GetImageHeader(); + if (image_header.IsAppImage() && image_space->HasAddress(klass_ptr.Ptr())) { + return image_header.GetObjectsSection().Size(); + } + } + } + return 0; +} + +} // namespace + +} // namespace art diff --git a/test/1001-app-image-regions/build b/test/1001-app-image-regions/build new file mode 100755 index 0000000000..16c3a5b566 --- /dev/null +++ b/test/1001-app-image-regions/build @@ -0,0 +1,34 @@ +#!/bin/bash +# +# Copyright 2019 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# make us exit on a failure +set -e + +count=10000 +echo "LMain;" >> profile +for i in $(seq 1 "$count"); do + echo "LOther\$Inner${i};" >> "profile" +done + +# Generate the other class. +other_file="src/Other.java" +echo "class Other {" >> "${other_file}" +for i in $(seq 1 "$count"); do + echo " static class Inner${i} { void test(){} }" >> "${other_file}" +done +echo "}" >> "${other_file}" + +./default-build "$@" diff --git a/test/1001-app-image-regions/expected.txt b/test/1001-app-image-regions/expected.txt new file mode 100644 index 0000000000..9fd87cad85 --- /dev/null +++ b/test/1001-app-image-regions/expected.txt @@ -0,0 +1,4 @@ +JNI_OnLoad called +App image loaded true +Region size 262144 +App image section size large enough true diff --git a/test/1001-app-image-regions/info.txt b/test/1001-app-image-regions/info.txt new file mode 100644 index 0000000000..b13f2768cd --- /dev/null +++ b/test/1001-app-image-regions/info.txt @@ -0,0 +1 @@ +Tests that an app image with many classes is generated and loaded correctly. diff --git a/test/1001-app-image-regions/run b/test/1001-app-image-regions/run new file mode 100644 index 0000000000..128aa2e709 --- /dev/null +++ b/test/1001-app-image-regions/run @@ -0,0 +1,17 @@ +#!/bin/bash +# +# Copyright (C) 2019 The Android Open Source Project +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +exec ${RUN} $@ --profile -Xcompiler-option --compiler-filter=speed-profile diff --git a/test/1001-app-image-regions/src/Main.java b/test/1001-app-image-regions/src/Main.java new file mode 100644 index 0000000000..c41a6069bc --- /dev/null +++ b/test/1001-app-image-regions/src/Main.java @@ -0,0 +1,37 @@ +/* + * Copyright 2019 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +import java.lang.reflect.Field; +import java.lang.reflect.Constructor; +import java.lang.reflect.Method; + +class Main { + public static void main(String[] args) { + System.loadLibrary(args[0]); + System.out.println("App image loaded " + checkAppImageLoaded()); + int regionSize = getRegionSize(); + int objectsSectionSize = checkAppImageSectionSize(Main.class); + System.out.println("Region size " + regionSize); + System.out.println("App image section size large enough " + (objectsSectionSize > regionSize)); + if (objectsSectionSize <= regionSize) { + System.out.println("Section size " + objectsSectionSize); + } + } + + public static native boolean checkAppImageLoaded(); + public static native int getRegionSize(); + public static native int checkAppImageSectionSize(Class c); +} diff --git a/test/Android.bp b/test/Android.bp index 57fcd86875..467a7179a5 100644 --- a/test/Android.bp +++ b/test/Android.bp @@ -495,6 +495,7 @@ cc_defaults { "708-jit-cache-churn/jit.cc", "800-smali/jni.cc", "909-attach-agent/disallow_debugging.cc", + "1001-app-image-regions/app_image_regions.cc", "1947-breakpoint-redefine-deopt/check_deopt.cc", "common/runtime_state.cc", "common/stack_inspect.cc", diff --git a/test/knownfailures.json b/test/knownfailures.json index 983c16a051..9c01ba9c55 100644 --- a/test/knownfailures.json +++ b/test/knownfailures.json @@ -81,11 +81,13 @@ "variant": "no-prebuild" }, { - "tests": ["118-noimage-dex2oat"], + "tests": ["118-noimage-dex2oat", + "1001-app-image-regions"], "variant": "no-relocate", "description": ["118-noimage-dex2oat is not broken per-se it just ", "doesn't work (and isn't meant to) without --prebuild ", - "--relocate"] + "--relocate. 1001-app-image-regions is disabled since it", + "doesn't have the app image loaded for no-relocate"] }, { "tests" : "629-vdex-speed", @@ -1085,6 +1087,7 @@ "688-shared-library", "999-redefine-hiddenapi", "1000-non-moving-space-stress", + "1001-app-image-regions", "1951-monitor-enter-no-suspend", "1957-error-ext"], "variant": "jvm", |