diff options
| -rw-r--r-- | compiler/driver/compiler_driver.cc | 28 | ||||
| -rw-r--r-- | runtime/class_linker.cc | 75 | ||||
| -rw-r--r-- | test/596-app-images/src/Main.java | 88 |
3 files changed, 162 insertions, 29 deletions
diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc index 70c3f6098a..3fdfb31b9b 100644 --- a/compiler/driver/compiler_driver.cc +++ b/compiler/driver/compiler_driver.cc @@ -2424,33 +2424,9 @@ class InitializeClassVisitor : public CompilationVisitor { } } - bool NoPotentialInternStrings(Handle<mirror::Class> klass, - Handle<mirror::ClassLoader>* class_loader) - REQUIRES_SHARED(Locks::mutator_lock_) { - StackHandleScope<1> hs(Thread::Current()); - Handle<mirror::DexCache> h_dex_cache = hs.NewHandle(klass->GetDexCache()); - const DexFile* dex_file = h_dex_cache->GetDexFile(); - const DexFile::ClassDef* class_def = klass->GetClassDef(); - annotations::RuntimeEncodedStaticFieldValueIterator value_it(*dex_file, - &h_dex_cache, - class_loader, - manager_->GetClassLinker(), - *class_def); - - const auto jString = annotations::RuntimeEncodedStaticFieldValueIterator::kString; - for ( ; value_it.HasNext(); value_it.Next()) { - if (value_it.GetValueType() == jString) { - // We don't want cache the static encoded strings which is a potential intern. - return false; - } - } - - return true; - } - bool ResolveTypesOfMethods(Thread* self, ArtMethod* m) REQUIRES_SHARED(Locks::mutator_lock_) { - auto rtn_type = m->GetReturnType(true); // return value is discarded because resolve will be done internally. + auto rtn_type = m->GetReturnType(true); if (rtn_type == nullptr) { self->ClearException(); return false; @@ -2575,7 +2551,7 @@ class InitializeClassVisitor : public CompilationVisitor { return false; } - return NoPotentialInternStrings(klass, class_loader); + return true; } const ParallelCompilationManager* const manager_; diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc index cffdfd3410..81ca764523 100644 --- a/runtime/class_linker.cc +++ b/runtime/class_linker.cc @@ -55,6 +55,7 @@ #include "gc_root-inl.h" #include "gc/accounting/card_table-inl.h" #include "gc/accounting/heap_bitmap-inl.h" +#include "gc/accounting/space_bitmap-inl.h" #include "gc/heap.h" #include "gc/scoped_gc_critical_section.h" #include "gc/space/image_space.h" @@ -88,6 +89,7 @@ #include "mirror/method_handles_lookup.h" #include "mirror/object-inl.h" #include "mirror/object_array-inl.h" +#include "mirror/object-refvisitor-inl.h" #include "mirror/proxy.h" #include "mirror/reference-inl.h" #include "mirror/stack_trace_element.h" @@ -1192,6 +1194,63 @@ class VerifyDeclaringClassVisitor : public ArtMethodVisitor { gc::accounting::HeapBitmap* const live_bitmap_; }; +class FixupInternVisitor { + public: + ALWAYS_INLINE ObjPtr<mirror::Object> TryInsertIntern(mirror::Object* obj) const + REQUIRES_SHARED(Locks::mutator_lock_) { + if (obj != nullptr && obj->IsString()) { + const auto intern = Runtime::Current()->GetInternTable()->InternStrong(obj->AsString()); + return intern; + } + return obj; + } + + ALWAYS_INLINE void VisitRootIfNonNull( + mirror::CompressedReference<mirror::Object>* root) const + REQUIRES_SHARED(Locks::mutator_lock_) { + if (!root->IsNull()) { + VisitRoot(root); + } + } + + ALWAYS_INLINE void VisitRoot(mirror::CompressedReference<mirror::Object>* root) const + REQUIRES_SHARED(Locks::mutator_lock_) { + root->Assign(TryInsertIntern(root->AsMirrorPtr())); + } + + // Visit Class Fields + ALWAYS_INLINE void operator()(ObjPtr<mirror::Object> obj, + MemberOffset offset, + bool is_static ATTRIBUTE_UNUSED) const + REQUIRES_SHARED(Locks::mutator_lock_) { + // There could be overlap between ranges, we must avoid visiting the same reference twice. + // Avoid the class field since we already fixed it up in FixupClassVisitor. + if (offset.Uint32Value() != mirror::Object::ClassOffset().Uint32Value()) { + // Updating images, don't do a read barrier. + // Only string fields are fixed, don't do a verify. + mirror::Object* ref = obj->GetFieldObject<mirror::Object, kVerifyNone, kWithoutReadBarrier>( + offset); + obj->SetFieldObject<false, false>(offset, TryInsertIntern(ref)); + } + } + + void operator()(ObjPtr<mirror::Class> klass ATTRIBUTE_UNUSED, + ObjPtr<mirror::Reference> ref) const + REQUIRES_SHARED(Locks::mutator_lock_) REQUIRES(Locks::heap_bitmap_lock_) { + this->operator()(ref, mirror::Reference::ReferentOffset(), false); + } + + void operator()(mirror::Object* obj) const + REQUIRES_SHARED(Locks::mutator_lock_) { + if (obj->IsDexCache()) { + obj->VisitReferences<true, kVerifyNone, kWithoutReadBarrier>(*this, *this); + } else { + // Don't visit native roots for non-dex-cache + obj->VisitReferences<false, kVerifyNone, kWithoutReadBarrier>(*this, *this); + } + } +}; + // Copies data from one array to another array at the same position // if pred returns false. If there is a page of continuous data in // the src array for which pred consistently returns true then @@ -1284,6 +1343,7 @@ bool AppImageClassLoadersAndDexCachesHelper::Update( return false; } } + // Only add the classes to the class loader after the points where we can return false. for (size_t i = 0; i < num_dex_caches; i++) { ObjPtr<mirror::DexCache> dex_cache = dex_caches->Get(i); @@ -1447,6 +1507,21 @@ bool AppImageClassLoadersAndDexCachesHelper::Update( } } } + { + // Fixup all the literal strings happens at app images which are supposed to be interned. + ScopedTrace timing("Fixup String Intern in image and dex_cache"); + const auto& image_header = space->GetImageHeader(); + const auto bitmap = space->GetMarkBitmap(); // bitmap of objects + const uint8_t* target_base = space->GetMemMap()->Begin(); + const ImageSection& objects_section = + image_header.GetImageSection(ImageHeader::kSectionObjects); + + uintptr_t objects_begin = reinterpret_cast<uintptr_t>(target_base + objects_section.Offset()); + uintptr_t objects_end = reinterpret_cast<uintptr_t>(target_base + objects_section.End()); + + FixupInternVisitor fixup_intern_visitor; + bitmap->VisitMarkedRange(objects_begin, objects_end, fixup_intern_visitor); + } if (*out_forward_dex_cache_array) { ScopedTrace timing("Fixup ArtMethod dex cache arrays"); FixupArtMethodArrayVisitor visitor(header); diff --git a/test/596-app-images/src/Main.java b/test/596-app-images/src/Main.java index 8ee3c888b0..674ba4d037 100644 --- a/test/596-app-images/src/Main.java +++ b/test/596-app-images/src/Main.java @@ -14,6 +14,10 @@ * limitations under the License. */ +import java.lang.reflect.Field; +import java.lang.reflect.Constructor; +import java.lang.reflect.Method; + class Main { static class Inner { final public static int abc = 10; @@ -46,13 +50,76 @@ class Main { if (!checkInitialized(StaticFieldsInit.class)) System.out.println("StaticFieldsInit class is not initialized!"); - if (checkInitialized(StaticInternString.class)) - System.out.println("StaticInternString class is initialized!"); + if (!checkInitialized(StaticInternString.class)) + System.out.println("StaticInternString class is not initialized!"); + + StringBuffer sb = new StringBuffer(); + sb.append("java."); + sb.append("abc."); + sb.append("Action"); + + String tmp = sb.toString(); + String intern = tmp.intern(); + + assertNotEqual(tmp, intern, "Dynamically constructed String, not interned."); + assertEqual(intern, StaticInternString.intent, "Static encoded literal String not interned."); + assertEqual(BootInternedString.boot, BootInternedString.boot.intern(), + "Static encoded literal String not moved back to runtime intern table."); + + try { + Field f = StaticInternString.class.getDeclaredField("intent"); + assertEqual(intern, f.get(null), "String Literals are not interned properly."); + + } catch (Exception e) { + System.out.println("Exception"); + } + + assertEqual(StaticInternString.getIntent(), StaticInternString2.getIntent(), + "String Literals are not intenred properly, App image static strings duplicated."); + + // reload the class StaticInternString, check whether static strings interned properly + final String DEX_FILE = System.getenv("DEX_LOCATION") + "/596-app-images.jar"; + final String LIBRARY_SEARCH_PATH = System.getProperty("java.library.path"); + + try { + Class<?> pathClassLoader = Class.forName("dalvik.system.PathClassLoader"); + if (pathClassLoader == null) { + throw new AssertionError("Counldn't find path class loader class"); + } + Constructor<?> ctor = + pathClassLoader.getDeclaredConstructor(String.class, String.class, ClassLoader.class); + ClassLoader loader = (ClassLoader) ctor.newInstance( + DEX_FILE, LIBRARY_SEARCH_PATH, null); + + Class<?> staticInternString = loader.loadClass("StaticInternString"); + + if (!checkAppImageContains(staticInternString)) { + System.out.println("Not loaded again."); + } + Method getIntent = staticInternString.getDeclaredMethod("getIntent"); + + assertEqual(StaticInternString.getIntent(), getIntent.invoke(staticInternString), + "Dynamically loaded app image's literal strings not interned properly."); + } catch (Exception e) { + e.printStackTrace(); + } + } public static native boolean checkAppImageLoaded(); public static native boolean checkAppImageContains(Class<?> klass); public static native boolean checkInitialized(Class<?> klass); + + public static void assertEqual(Object a, Object b, String msg) { + if (a != b) + System.out.println(msg); + } + + public static void assertNotEqual(Object a, Object b, String msg) { + if (a == b) + System.out.println(msg); + } + } class StaticFields{ @@ -68,6 +135,21 @@ class StaticFieldsInit{ } class StaticInternString { - final public static String intern = "java.abc.Action"; + final public static String intent = "java.abc.Action"; + static public String getIntent() { + return intent; + } +} + +class BootInternedString { + final public static String boot = "double"; +} + +class StaticInternString2 { + final public static String intent = "java.abc.Action"; + + static String getIntent() { + return intent; + } } |