Add region padding for app images
For app images, partition the image such that no object spans cross
region boundaries.
Bug: 116059983
Bug: 116874661
Test: test-art-host
Change-Id: Iedffe9fac4b9b59f81de7dd607030ad3a8bcb602
diff --git a/dex2oat/linker/image_writer.cc b/dex2oat/linker/image_writer.cc
index 1331fc3..7733cb7 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 @@
}
// 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 @@
}
}
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 @@
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::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 @@
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 @@
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 @@
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 @@
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 b680265..8896e07 100644
--- a/dex2oat/linker/image_writer.h
+++ b/dex2oat/linker/image_writer.h
@@ -279,7 +279,7 @@
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 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 @@
// 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 0000000..dc16a84
--- /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 0000000..16c3a5b
--- /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 0000000..9fd87ca
--- /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 0000000..b13f276
--- /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 0000000..128aa2e
--- /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 0000000..c41a606
--- /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 57fcd86..467a717 100644
--- a/test/Android.bp
+++ b/test/Android.bp
@@ -495,6 +495,7 @@
"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 983c16a..9c01ba9 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",