Index: vm/vmcore/include/vm_strings.h =================================================================== --- vm/vmcore/include/vm_strings.h (revision 513697) +++ vm/vmcore/include/vm_strings.h (working copy) @@ -37,12 +37,6 @@ extern "C" { #endif -/** - * Interns a string. - */ -VMEXPORT jstring -string_intern(JNIEnv*, jobject string); - #ifdef __cplusplus } #endif Index: vm/vmcore/include/String_Pool.h =================================================================== --- vm/vmcore/include/String_Pool.h (revision 513697) +++ vm/vmcore/include/String_Pool.h (working copy) @@ -35,15 +35,9 @@ #ifdef _DEBUG unsigned id; // id for debugging #endif - // 20030507 Ref to the interned string used by java.lang.String.intern(). - // It is compressed when compressing refs. unsigned len; - union { - // raw reference to interned string if not compressing references - ManagedObject * raw_ref; - // equivalent compressed reference. - uint32 compressed_ref; - } intern; + // reference to interned string + ManagedObject* ref; char bytes[STRING_PADDING]; }; @@ -61,7 +55,7 @@ String * get_next_string_intern(); // intern the string - ManagedObject * intern(String *); + ManagedObject* intern(String *); // The GC enumeration code needs to lock and unlock the // pool in order to enumerate it in a thread safe manner. @@ -122,8 +116,9 @@ volatile Interned_Strings * current_interned; // iterator to go through interned strings // NOTE: only one thread can iterate through at once - // Moreover the iteration should be done in stop_the_world phase since no - // synchronization between String_Pool::intern() & String_Pool::get_next_string_intern() + // Moreover the iteration should be done in + // stop_the_world phase since no synchronization between + // vm_instantiate_cp_string_resolved & String_Pool::get_next_string_intern() Interned_Strings_Index * index_interned; // In order to make this code thread safe a light weight lock is used. Index: vm/vmcore/include/environment.h =================================================================== --- vm/vmcore/include/environment.h (revision 513697) +++ vm/vmcore/include/environment.h (working copy) @@ -156,6 +156,12 @@ String* InitCauseDescriptor_String; /** + * Preloaded methods + */ + Method* VM_intern; + + + /** * Preloaded classes */ Class* Boolean_Class; Index: vm/vmcore/src/gc/root_set_enum_common.cpp =================================================================== --- vm/vmcore/src/gc/root_set_enum_common.cpp (revision 513697) +++ vm/vmcore/src/gc/root_set_enum_common.cpp (working copy) @@ -41,23 +41,12 @@ String *ps = VM_Global_State::loader_env->string_pool.get_first_string_intern(); // 20030405 Don't enumerate references that are *unmanaged null* (i.e. zero/NULL) // since vm_enumerate_root_reference() expects to be called with slots containing managed refs. - if (VM_Global_State::loader_env->compress_references) { - while (ps != NULL) { - COMPRESSED_REFERENCE compressed_ref = ps->intern.compressed_ref; - assert(is_compressed_reference(compressed_ref)); - assert(compressed_ref != 0); - vm_enumerate_compressed_root_reference((COMPRESSED_REFERENCE *)&ps->intern.compressed_ref, - VM_Global_State::loader_env->pin_interned_strings); - ps = VM_Global_State::loader_env->string_pool.get_next_string_intern(); - } - } else { - while (ps != NULL) { - ManagedObject* s = ps->intern.raw_ref; - assert(s != NULL); - vm_enumerate_root_reference((void **)&(ps->intern.raw_ref), - VM_Global_State::loader_env->pin_interned_strings); - ps = VM_Global_State::loader_env->string_pool.get_next_string_intern(); - } + while(ps != NULL) { + ManagedObject* s = ps->ref; + assert(s != NULL); + vm_enumerate_root_reference((void **)&(ps->ref), + VM_Global_State::loader_env->pin_interned_strings); + ps = VM_Global_State::loader_env->string_pool.get_next_string_intern(); } } //vm_enumerate_interned_strings Index: vm/vmcore/src/kernel_classes/native/org_apache_harmony_kernel_vm_VM.cpp =================================================================== --- vm/vmcore/src/kernel_classes/native/org_apache_harmony_kernel_vm_VM.cpp (revision 513697) +++ vm/vmcore/src/kernel_classes/native/org_apache_harmony_kernel_vm_VM.cpp (working copy) @@ -38,10 +38,3 @@ // reuse similar method in VMClassRegistry return Java_java_lang_VMClassRegistry_getClassLoader0(jenv, NULL, clazz); } - -JNIEXPORT jstring JNICALL -Java_org_apache_harmony_kernel_vm_VM_intern0(JNIEnv *jenv, jclass, jstring str) -{ - // call corresponding VM internal function - return string_intern(jenv, str); -} Index: vm/vmcore/src/kernel_classes/javasrc/org/apache/harmony/kernel/vm/VM.java =================================================================== --- vm/vmcore/src/kernel_classes/javasrc/org/apache/harmony/kernel/vm/VM.java (revision 513697) +++ vm/vmcore/src/kernel_classes/javasrc/org/apache/harmony/kernel/vm/VM.java (working copy) @@ -22,6 +22,8 @@ package org.apache.harmony.kernel.vm; import org.apache.harmony.vm.VMStack; +import java.lang.ref.WeakReference; +import java.util.WeakHashMap; public final class VM { @@ -96,7 +98,7 @@ public static void deleteOnExit() { deleteOnExit = true; } - + /** * Returns an intern-ed representation of the * String @@ -105,12 +107,18 @@ * @return String that has the same contents as * argument, but from internal pool */ - public static String intern(String s) { - return intern0(s); + public static synchronized String intern(String s) { + // NB: Since this function is called for each + // string literal instantiation, string literals MUST NOT be used + // in this function or functions called from here + + return internedStrings.intern(s); } - - /** - * Invokes native string interning service. - */ - private static native String intern0(String s); + + private static InternMap internedStrings; + + static { + // initialize the storage for interned strings + internedStrings = new InternMap(32768); + } } Index: vm/vmcore/src/kernel_classes/javasrc/org/apache/harmony/kernel/vm/InternMap.java =================================================================== --- vm/vmcore/src/kernel_classes/javasrc/org/apache/harmony/kernel/vm/InternMap.java (revision 0) +++ vm/vmcore/src/kernel_classes/javasrc/org/apache/harmony/kernel/vm/InternMap.java (revision 0) @@ -0,0 +1,145 @@ +/* Copyright 1998, 2005 The Apache Software Foundation or its licensors, as applicable + * + * 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. + */ + +package org.apache.harmony.kernel.vm; + +import java.lang.ref.ReferenceQueue; +import java.lang.ref.WeakReference; + +/** + * Implements weak hash map specialized for storing interned string pool. + * @see java.util.WeakHashMap + * @see WeakReference + */ +public class InternMap { + + private final ReferenceQueue referenceQueue; + + int elementCount; + + Entry[] elementData; + + private final int loadFactor; + + private int threshold; + + //Simple utility method to isolate unchecked cast for array creation + private static Entry[] newEntryArray(int size) { + return new Entry[size]; + } + + private static final class Entry extends WeakReference { + int hash; + Entry next; + Entry(String key, ReferenceQueue queue) { + super(key, queue); + hash = key.hashCode(); + } + } + + public InternMap(int capacity) { + if (capacity >= 0) { + elementCount = 0; + elementData = newEntryArray(capacity == 0 ? 1 : capacity); + loadFactor = 7500; // Default load factor of 0.75 + computeMaxSize(); + referenceQueue = new ReferenceQueue(); + } else { + throw new IllegalArgumentException(); + } + } + + private void computeMaxSize() { + threshold = (int) ((long) elementData.length * loadFactor / 10000); + } + + void poll() { + Entry toRemove; + while ((toRemove = (Entry)referenceQueue.poll()) != null) { + removeEntry(toRemove); + } + } + + void removeEntry(Entry toRemove) { + Entry entry, last = null; + int index = (toRemove.hash & 0x7FFFFFFF) % elementData.length; + entry = elementData[index]; + // Ignore queued entries which cannot be found, the user could + // have removed them before they were queued, i.e. using clear() + while (entry != null) { + if (toRemove == entry) { + if (last == null) { + elementData[index] = entry.next; + } else { + last.next = entry.next; + } + elementCount--; + break; + } + last = entry; + entry = entry.next; + } + } + + public String intern(String key) { + int index = 0; + Entry entry; + String interned = null; + if (key == null) + return null; + index = (key.hashCode() & 0x7FFFFFFF) % elementData.length; + entry = elementData[index]; + while (entry != null && !key.equals(interned = (String)entry.get())) { + entry = entry.next; + } + + // if we found the entry, return it + if (entry != null) { + return interned; + } + + // no interned string found, put a new entry for it + if (++elementCount > threshold) { + rehash(); + index = (key.hashCode() & 0x7FFFFFFF) % elementData.length; + } + entry = new Entry(key, referenceQueue); + entry.next = elementData[index]; + elementData[index] = entry; + return key; + } + + private void rehash() { + poll(); + int length = elementData.length << 1; + if (length == 0) { + length = 1; + } + Entry[] newData = newEntryArray(length); + for (int i = 0; i < elementData.length; i++) { + Entry entry = elementData[i]; + while (entry != null) { + int index = (entry.hash & 0x7FFFFFFF) % length; + Entry next = entry.next; + entry.next = newData[index]; + newData[index] = entry; + entry = next; + } + } + elementData = newData; + computeMaxSize(); + } +} + Index: vm/vmcore/src/class_support/String_Pool.cpp =================================================================== --- vm/vmcore/src/class_support/String_Pool.cpp (revision 513697) +++ vm/vmcore/src/class_support/String_Pool.cpp (working copy) @@ -19,30 +19,27 @@ * @version $Revision: 1.1.2.1.4.4 $ */ -#include "platform_lowlevel.h" - -//MVM -#include -using namespace std; - -#include #include #include #include #include -#include "String_Pool.h" -#include "environment.h" #include "open/hythread.h" #include "open/vm_util.h" #include "open/gc.h" + +#define LOG_DOMAIN "vm.strings" +#include "cxxlog.h" + +#include "platform_lowlevel.h" +#include "String_Pool.h" +#include "environment.h" #include "atomics.h" #include "vm_strings.h" #include "vm_stats.h" +#include "ini.h" +#include "exceptions.h" -#define LOG_DOMIAN "vm.strings" -#include "cxxlog.h" - // apr_atomic_casptr should result in an .acq on IPF. void String_Pool::lock_pool () { // Spin until lock is m_free. @@ -59,10 +56,7 @@ } //String_Pool::unlock_pool String_Pool::Entry::Entry(const char * s, size_t len, Entry *n) : next(n) { -// This constructor can be run very early on during VM execution--even before main is entered. -// This is before we have had a chance to process any command line arguments. So, initialize the -// interned Java_lang_String reference to NULL in a way that will work whether references are compressed or not. - str.intern.raw_ref = NULL; + str.ref = NULL; str.len = (unsigned)len; assert(strlen(s) >= len); memcpy(str.bytes, s, len); @@ -74,7 +68,7 @@ unsigned size = sizeof(Entry*) * STRING_TABLE_SIZE; table = (Entry **)memory_pool.alloc(size); memset(table, 0, size); - + head_interned = (Interned_Strings *)memory_pool.alloc(sizeof(Interned_Strings)); memset(head_interned, 0, sizeof(Interned_Strings)); current_interned = head_interned; @@ -300,9 +294,11 @@ } assert(current_interned->free_slot < INTERNED_STRING_ARRAY_SIZE); if (current_interned->free_slot == INTERNED_STRING_ARRAY_SIZE - 1) { - // this piece of code should be executed in one thread until current_interned is updated - volatile Interned_Strings * local_current_interned = current_interned; - Interned_Strings * new_elem = (Interned_Strings *)memory_pool.alloc(sizeof(Interned_Strings)); + // this piece of code should be executed in one thread + // until current_interned is updated + volatile Interned_Strings* local_current_interned = current_interned; + Interned_Strings* new_elem = + (Interned_Strings*)memory_pool.alloc(sizeof(Interned_Strings)); memset(new_elem, 0, sizeof(Interned_Strings)); current_interned->next = new_elem; MemoryWriteBarrier(); @@ -314,53 +310,39 @@ } } -// NOTE: it is safe to call this function in multiple threads BUT -// don't iterate through interned strings while other threads do interning -ManagedObject * String_Pool::intern(String * str) { - ManagedObject* lang_string = string_create_from_utf8(str->bytes, str->len); - - if (!lang_string) { // if OutOfMemory +ManagedObject * String_Pool::intern(String * str) +{ + jobject string = oh_allocate_local_handle(); + string->object = string_create_from_utf8(str->bytes, str->len); + if(!string->object) { // if OutOfMemory return NULL; } + assert(!hythread_is_suspend_enabled()); + Global_Env* env = VM_Global_State::loader_env; + jvalue args[1]; + args[0].l = string; + assert(env->VM_intern); + vm_execute_java_method_array((jmethodID)env->VM_intern, + (jvalue*)&string, args); + ASSERT(!exn_raised(), "interning resulted in exception"); + ASSERT(string, "interning '" << str->bytes << "' returned NULL"); + assert(string->object); - // Atomically update the string structure since some other thread might be trying to make the same update. - // The GC won't be able to enumerate here since GC is disabled, so there are no race conditions with GC. - if (VM_Global_State::loader_env->compress_references) { - COMPRESSED_REFERENCE compressed_lang_string = - (COMPRESSED_REFERENCE)((POINTER_SIZE_INT)lang_string - - (POINTER_SIZE_INT)VM_Global_State::loader_env->heap_base); - assert(is_compressed_reference(compressed_lang_string)); - uint32 result = apr_atomic_cas32( - /*destination*/ (volatile uint32 *)&str->intern.compressed_ref, - /*exchange*/ compressed_lang_string, - /*comparand*/ 0); - if (result == 0) { - // Note the successful write of the object. - gc_heap_write_global_slot_compressed( - (COMPRESSED_REFERENCE *)&str->intern.compressed_ref, - (Managed_Object_Handle)lang_string); - // add this string to interned strings - register_interned_string(str); - } - // Some other thread may have beaten us to the slot. - lang_string = (ManagedObject *)uncompress_compressed_reference(str->intern.compressed_ref); - } else { - void *result = - (void *)apr_atomic_casptr( - /*destination*/ (volatile void **)&str->intern.raw_ref, - /*exchange*/ (void *)lang_string, - /*comparand*/ (void *)NULL); - if (result == NULL) { - // Note the successful write of the object. - gc_heap_write_global_slot( - (Managed_Object_Handle *)&str->intern.raw_ref, - (Managed_Object_Handle)lang_string); - // add this string to interned strings - register_interned_string(str); - } - // Some other thread may have beaten us to the slot. - lang_string = str->intern.raw_ref; - } - return lang_string; + // Cache the strong reference to interned string in string pool. + // Many threads may run the same string interning at once, + // but since they get the same value from VM.intern(), + // the unsynchronized store looks to be safe. + str->ref = string->object; + // Note the write of the object. + gc_heap_write_global_slot((Managed_Object_Handle*)&(str->ref), + (Managed_Object_Handle)str->ref); + register_interned_string(str); + + oh_discard_local_handle(string); + + TRACE2("intern", "interning literal '" << str->bytes << "' to " + << (void*)string->object); + + return (Java_java_lang_String*)str->ref; } Index: vm/vmcore/src/class_support/C_Interface.cpp =================================================================== --- vm/vmcore/src/class_support/C_Interface.cpp (revision 513697) +++ vm/vmcore/src/class_support/C_Interface.cpp (working copy) @@ -752,14 +752,7 @@ String* str = cl->get_constant_pool().get_string(index); assert(str); - bool must_instantiate; - if (env->compress_references) { - must_instantiate = (str->intern.compressed_ref == 0 /*NULL*/); - } else { - must_instantiate = (str->intern.raw_ref == NULL); - } - - if (must_instantiate) { + if (str->ref == NULL) { // vm_instantiate_cp_string_resolved assumes that GC is disabled tmn_suspend_disable(); // Discard the result. We are only interested in the side-effect of setting str->intern. @@ -769,11 +762,7 @@ tmn_suspend_enable(); } - if (env->compress_references) { - return &(str->intern.compressed_ref); - } else { - return &(str->intern.raw_ref); - } + return &(str->ref); } //class_get_const_string_intern_addr Index: vm/vmcore/src/class_support/classloader.cpp =================================================================== --- vm/vmcore/src/class_support/classloader.cpp (revision 513697) +++ vm/vmcore/src/class_support/classloader.cpp (working copy) @@ -1590,11 +1590,7 @@ // can cause GC and we would like to get away with code that doesn't // protect references from GC. ManagedObject* jstr; - if (env->compress_references) { - jstr = uncompress_compressed_reference(class_name_with_dots->intern.compressed_ref); - } else { - jstr = class_name_with_dots->intern.raw_ref; - } + jstr = class_name_with_dots->ref; if (jstr != NULL) { ObjectHandle h = oh_allocate_local_handle(); h->object = jstr; Index: vm/vmcore/src/init/vm_init.cpp =================================================================== --- vm/vmcore/src/init/vm_init.cpp (revision 513697) +++ vm/vmcore/src/init/vm_init.cpp (working copy) @@ -370,6 +370,10 @@ (class_lookup_field_recursive(vm_env->JavaLangString_Class, "bvalue", "[B") != NULL); vm_env->JavaLangString_VTable = vm_env->JavaLangString_Class->get_vtable(); + Class* VM_class = preload_class(vm_env, "org/apache/harmony/kernel/vm/VM"); + vm_env->VM_intern = class_lookup_method_recursive(VM_class, "intern", + "(Ljava/lang/String;)Ljava/lang/String;"); + TRACE2("init", "preloading exceptions"); vm_env->java_lang_Throwable_Class = preload_class(vm_env, vm_env->JavaLangThrowable_String); Index: vm/vmcore/src/util/vm_stats.cpp =================================================================== --- vm/vmcore/src/util/vm_stats.cpp (revision 513697) +++ vm/vmcore/src/util/vm_stats.cpp (working copy) @@ -639,18 +639,11 @@ num_lookup_total += key_stats->num_lookup; num_lookup_collision_total += key_stats->num_lookup_collision; - + bool is_interned = false; String * string = string_pool.lookup(key, key_len); - if (VM_Global_State::loader_env->compress_references) { - if (string->intern.compressed_ref) { - is_interned = true; - } - } else { - if (string->intern.raw_ref) { - is_interned = true; - } - } + if(string->ref) + is_interned = true; char * str = key; // replace '\n' literal with space Index: vm/vmcore/src/util/vm_strings.cpp =================================================================== --- vm/vmcore/src/util/vm_strings.cpp (revision 513697) +++ vm/vmcore/src/util/vm_strings.cpp (working copy) @@ -542,41 +542,24 @@ /////////////////////////////////////////////////////////////////// // Old interface - -// Given a String, creates its interned Java_java_lang_string from its byte array. GC must be disabled +// Given a String, creates its interned Java_java_lang_string from its byte array. +// GC must be disabled +// NOTE: it is safe to call this function in multiple threads BUT +// don't iterate through interned strings while other threads do interning VMEXPORT // temporary solution for interpreter unplug -Java_java_lang_String *vm_instantiate_cp_string_resolved(String *str) +Java_java_lang_String* vm_instantiate_cp_string_resolved(String* str) { ASSERT_RAISE_AREA; assert(!hythread_is_suspend_enabled()); - Global_Env *env = VM_Global_State::loader_env; - if (env->compress_references) { - if (str->intern.compressed_ref != 0) { - return uncompress_compressed_reference(str->intern.compressed_ref); - } - } else { - if (str->intern.raw_ref != NULL) { - return str->intern.raw_ref; - } + if(str->ref != NULL) { + return str->ref; } - return env->string_pool.intern(str); -} //vm_instantiate_cp_string_resolved + return VM_Global_State::loader_env->string_pool.intern(str); +} // vm_instantiate_cp_string_resolved + // Interning of strings -VMEXPORT jstring string_intern(JNIEnv *jenv, jobject jstr_obj) -{ - ASSERT_RAISE_AREA; - jboolean is_copy; - const char* val = jenv->GetStringUTFChars(jstr_obj, &is_copy); - String* str = VM_Global_State::loader_env->string_pool.lookup(val); - if (is_copy == JNI_TRUE) { - jenv->ReleaseStringUTFChars(jstr_obj, val); - } - - return String_to_interned_jstring(str); -} - jstring String_to_interned_jstring(String* str) { ASSERT_RAISE_AREA; Index: vm/tests/smoke/stress/Intern.java =================================================================== --- vm/tests/smoke/stress/Intern.java (revision 0) +++ vm/tests/smoke/stress/Intern.java (revision 0) @@ -0,0 +1,37 @@ +/* + * Copyright 2005-2006 The Apache Software Foundation or its licensors, as applicable. + * + * 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. + */ +package stress; + +/** + * Tests the correctness of string interning. + * + * @keyword XXX_stress + */ +public class Intern { + public static void main(String[] args) { + String s = "abc"; + for (int i = 0; i < 100000; i++) { + s = (s + i + s).intern(); + if (s.length() > 65536) s = "abc" + i; + if (i % 1000 == 0) trace("."); + } + } + + public static void trace(Object o) { + System.out.print(o); + System.out.flush(); + } +}