Merge "Revert "Fixed Literal String intern mismatch."" am: 61c42e94af
am: 7c2f29f774
Change-Id: I465eaf2e2aebf4acaadf537979d80cd453b3d97e
diff --git a/compiler/driver/compiler_driver.cc b/compiler/driver/compiler_driver.cc
index 0097f55..d008060 100644
--- a/compiler/driver/compiler_driver.cc
+++ b/compiler/driver/compiler_driver.cc
@@ -2426,9 +2426,33 @@
}
}
+ 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);
+ auto rtn_type = m->GetReturnType(true); // return value is discarded because resolve will be done internally.
if (rtn_type == nullptr) {
self->ClearException();
return false;
@@ -2553,7 +2577,7 @@
return false;
}
- return true;
+ return NoPotentialInternStrings(klass, class_loader);
}
const ParallelCompilationManager* const manager_;
diff --git a/runtime/class_linker.cc b/runtime/class_linker.cc
index a81c832..0921bd6 100644
--- a/runtime/class_linker.cc
+++ b/runtime/class_linker.cc
@@ -55,7 +55,6 @@
#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"
@@ -89,7 +88,6 @@
#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"
@@ -1195,63 +1193,6 @@
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
@@ -1344,7 +1285,6 @@
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);
@@ -1508,21 +1448,6 @@
}
}
}
- {
- // 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 88d95f4..8ee3c88 100644
--- a/test/596-app-images/src/Main.java
+++ b/test/596-app-images/src/Main.java
@@ -14,10 +14,6 @@
* 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;
@@ -50,76 +46,13 @@
if (!checkInitialized(StaticFieldsInit.class))
System.out.println("StaticFieldsInit class is not 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(System.out);
- }
-
+ if (checkInitialized(StaticInternString.class))
+ System.out.println("StaticInternString class is initialized!");
}
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{
@@ -135,21 +68,6 @@
}
class StaticInternString {
- 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;
- }
+ final public static String intern = "java.abc.Action";
}