Fixed Literal String intern mismatch.
Literal Strings in app images are interned into local intern tables
which causes mismatch with the runtime intern table, especially when
a string is loaded from app images twice. Now when .art is loaded, a
visitor go through all classes and their fields to intern every string
literal found again.
Test case 596 is updated to test the string intern functioanlity.
Test on devices Nexus 5X, fixing strings takes 5.4 ms on GoogleMap
during startup.
Bug: 62224799
Test: test-art-host -j64
Change-Id: I2e1d44a79db1ae5f9aec80f228128201d1d838d8
diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc
index 70c3f60..3fdfb31 100644
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -2424,33 +2424,9 @@
}
}
- 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 @@
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 694c113..52bd86b 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"
@@ -1191,6 +1193,63 @@
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
@@ -1262,6 +1321,7 @@
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);
@@ -1425,6 +1485,21 @@
}
}
}
+ {
+ // 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 8ee3c88..674ba4d 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 @@
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 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;
+ }
}