# HG changeset patch # User rehn # Date 1543399587 -3600 # Node ID b22da519f2e3223fca4e0b3561e721cece8e7493 # Parent 877dd2b0f36c32025bdedd67dab1d6e63f59a3b4 8213791: StringTable: Use get and insert Reviewed-by: eosterlund, gziemski diff -r 877dd2b0f36c -r b22da519f2e3 src/hotspot/share/classfile/stringTable.cpp --- a/src/hotspot/share/classfile/stringTable.cpp Wed Nov 28 11:06:58 2018 +0100 +++ b/src/hotspot/share/classfile/stringTable.cpp Wed Nov 28 11:06:27 2018 +0100 @@ -206,6 +206,12 @@ _local_table = new StringTableHash(start_size_log_2, END_SIZE, REHASH_LEN); } +void StringTable::update_needs_rehash(bool rehash) { + if (rehash) { + _needs_rehashing = true; + } +} + size_t StringTable::item_added() { return Atomic::add((size_t)1, &(the_table()->_items_count)); } @@ -281,9 +287,7 @@ StringTableGet stg(thread); bool rehash_warning; _local_table->get(thread, lookup, stg, &rehash_warning); - if (rehash_warning) { - _needs_rehashing = true; - } + update_needs_rehash(rehash_warning); return stg.get_res_oop(); } @@ -334,30 +338,6 @@ hash, CHECK_NULL); } -class StringTableCreateEntry : public StackObj { - private: - Thread* _thread; - Handle _return; - Handle _store; - public: - StringTableCreateEntry(Thread* thread, Handle store) - : _thread(thread), _store(store) {} - - WeakHandle operator()() { // No dups found - WeakHandle wh = - WeakHandle::create(_store); - return wh; - } - void operator()(bool inserted, WeakHandle* val) { - oop result = val->resolve(); - assert(result != NULL, "Result should be reachable"); - _return = Handle(_thread, result); - } - oop get_return() const { - return _return(); - } -}; - oop StringTable::do_intern(Handle string_or_null_h, const jchar* name, int len, uintx hash, TRAPS) { HandleMark hm(THREAD); // cleanup strings created @@ -377,15 +357,23 @@ assert(java_lang_String::equals(string_h(), name, len), "string must be properly initialized"); assert(len == java_lang_String::length(string_h()), "Must be same length"); + StringTableLookupOop lookup(THREAD, hash, string_h); - StringTableCreateEntry stc(THREAD, string_h); + StringTableGet stg(THREAD); bool rehash_warning; - _local_table->get_insert_lazy(THREAD, lookup, stc, stc, &rehash_warning); - if (rehash_warning) { - _needs_rehashing = true; - } - return stc.get_return(); + do { + if (_local_table->get(THREAD, lookup, stg, &rehash_warning)) { + update_needs_rehash(rehash_warning); + return stg.get_res_oop(); + } + WeakHandle wh = WeakHandle::create(string_h); + // The hash table takes ownership of the WeakHandle, even if it's not inserted. + if (_local_table->insert(THREAD, lookup, wh, &rehash_warning)) { + update_needs_rehash(rehash_warning); + return wh.resolve(); + } + } while(true); } // GC support diff -r 877dd2b0f36c -r b22da519f2e3 src/hotspot/share/classfile/stringTable.hpp --- a/src/hotspot/share/classfile/stringTable.hpp Wed Nov 28 11:06:58 2018 +0100 +++ b/src/hotspot/share/classfile/stringTable.hpp Wed Nov 28 11:06:27 2018 +0100 @@ -93,6 +93,7 @@ void try_rehash_table(); bool do_rehash(); + inline void update_needs_rehash(bool rehash); public: // The string table diff -r 877dd2b0f36c -r b22da519f2e3 src/hotspot/share/utilities/concurrentHashTable.hpp --- a/src/hotspot/share/utilities/concurrentHashTable.hpp Wed Nov 28 11:06:58 2018 +0100 +++ b/src/hotspot/share/utilities/concurrentHashTable.hpp Wed Nov 28 11:06:27 2018 +0100 @@ -313,8 +313,13 @@ // Insert which handles a number of cases. template - bool internal_insert(Thread* thread, LOOKUP_FUNC& lookup_f, VALUE_FUNC& value_f, - CALLBACK_FUNC& callback, bool* grow_hint = NULL, bool* clean_hint = NULL); + bool internal_get_insert(Thread* thread, LOOKUP_FUNC& lookup_f, VALUE_FUNC& value_f, + CALLBACK_FUNC& callback_f, bool* grow_hint = NULL, bool* clean_hint = NULL); + + // Plain insert. + template + bool internal_insert(Thread* thread, LOOKUP_FUNC& lookup_f, const VALUE& value, + bool* grow_hint, bool* clean_hint); // Returns true if an item matching LOOKUP_FUNC is removed. // Calls DELETE_FUNC before destroying the node. @@ -402,7 +407,7 @@ template bool get_insert_lazy(Thread* thread, LOOKUP_FUNC& lookup_f, VALUE_FUNC& val_f, CALLBACK_FUNC& callback_f, bool* grow_hint = NULL, bool* clean_hint = NULL) { - return !internal_insert(thread, lookup_f, val_f, callback_f, grow_hint, clean_hint); + return !internal_get_insert(thread, lookup_f, val_f, callback_f, grow_hint, clean_hint); } // Same without CALLBACK_FUNC. @@ -442,8 +447,7 @@ template bool insert(Thread* thread, LOOKUP_FUNC& lookup_f, const VALUE& value, bool* grow_hint = NULL, bool* clean_hint = NULL) { - LazyValueRetrieve vp(value); - return internal_insert(thread, lookup_f, vp, noOp, grow_hint, clean_hint); + return internal_insert(thread, lookup_f, value, grow_hint, clean_hint); } // This does a fast unsafe insert and can thus only be used when there is no diff -r 877dd2b0f36c -r b22da519f2e3 src/hotspot/share/utilities/concurrentHashTable.inline.hpp --- a/src/hotspot/share/utilities/concurrentHashTable.inline.hpp Wed Nov 28 11:06:58 2018 +0100 +++ b/src/hotspot/share/utilities/concurrentHashTable.inline.hpp Wed Nov 28 11:06:27 2018 +0100 @@ -876,8 +876,8 @@ template template inline bool ConcurrentHashTable:: - internal_insert(Thread* thread, LOOKUP_FUNC& lookup_f, VALUE_FUNC& value_f, - CALLBACK_FUNC& callback, bool* grow_hint, bool* clean_hint) + internal_get_insert(Thread* thread, LOOKUP_FUNC& lookup_f, VALUE_FUNC& value_f, + CALLBACK_FUNC& callback_f, bool* grow_hint, bool* clean_hint) { bool ret = false; bool clean = false; @@ -901,7 +901,7 @@ new_node->set_next(first_at_start); } if (bucket->cas_first(new_node, first_at_start)) { - callback(true, new_node->value()); + callback_f(true, new_node->value()); new_node = NULL; ret = true; break; /* leave critical section */ @@ -910,7 +910,7 @@ locked = bucket->is_locked(); } else { // There is a duplicate. - callback(false, old->value()); + callback_f(false, old->value()); break; /* leave critical section */ } } /* leave critical section */ @@ -946,6 +946,70 @@ } template +template +inline bool ConcurrentHashTable:: + internal_insert(Thread* thread, LOOKUP_FUNC& lookup_f, const VALUE& value, + bool* grow_hint, bool* clean_hint) +{ + bool ret = false; + bool clean = false; + bool locked; + size_t loops = 0; + size_t i = 0; + uintx hash = lookup_f.get_hash(); + Node* new_node = Node::create_node(value, NULL); + + while (true) { + { + ScopedCS cs(thread, this); /* protected the table/bucket */ + Bucket* bucket = get_bucket(hash); + Node* first_at_start = bucket->first(); + Node* old = get_node(bucket, lookup_f, &clean, &loops); + if (old == NULL) { + new_node->set_next(first_at_start); + if (bucket->cas_first(new_node, first_at_start)) { + new_node = NULL; + ret = true; + break; /* leave critical section */ + } + // CAS failed we must leave critical section and retry. + locked = bucket->is_locked(); + } else { + // There is a duplicate. + break; /* leave critical section */ + } + } /* leave critical section */ + i++; + if (locked) { + os::naked_yield(); + } else { + SpinPause(); + } + } + + if (new_node != NULL) { + // CAS failed and a duplicate was inserted, we must free this node. + Node::destroy_node(new_node); + } else if (i == 0 && clean) { + // We only do cleaning on fast inserts. + Bucket* bucket = get_bucket_locked(thread, lookup_f.get_hash()); + delete_in_bucket(thread, bucket, lookup_f); + bucket->unlock(); + clean = false; + } + + if (grow_hint != NULL) { + *grow_hint = loops > _grow_hint; + } + + if (clean_hint != NULL) { + *clean_hint = clean; + } + + return ret; +} + +template template inline bool ConcurrentHashTable:: visit_nodes(Bucket* bucket, FUNC& visitor_f)