8215155: Remove get_insert() from concurrent hashtable and gtests
Summary: Replaced get_insert() with get()/insert() in gtest, removed get_insert() API from cht implementation.
Reviewed-by: coleenp, rehn
--- a/src/hotspot/share/utilities/concurrentHashTable.hpp Thu Jan 10 16:50:26 2019 +0100
+++ b/src/hotspot/share/utilities/concurrentHashTable.hpp Thu Jan 10 11:16:17 2019 -0600
@@ -312,11 +312,6 @@
VALUE* internal_get(Thread* thread, LOOKUP_FUNC& lookup_f,
bool* grow_hint = NULL);
- // Insert which handles a number of cases.
- template <typename LOOKUP_FUNC, typename VALUE_FUNC, typename CALLBACK_FUNC>
- 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 <typename LOOKUP_FUNC>
bool internal_insert(Thread* thread, LOOKUP_FUNC& lookup_f, const VALUE& value,
@@ -402,37 +397,6 @@
// methods on the table during a callback is not supported.Only MultiGetHandle
// supports multiple gets.
- // LOOKUP_FUNC is matching methods, VALUE_FUNC creates value to be inserted
- // and CALLBACK_FUNC is called with new or old value. Returns true if the
- // value already exists.
- template <typename LOOKUP_FUNC, typename VALUE_FUNC, typename CALLBACK_FUNC>
- 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_get_insert(thread, lookup_f, val_f, callback_f, grow_hint, clean_hint);
- }
-
- // Same without CALLBACK_FUNC.
- template <typename LOOKUP_FUNC, typename VALUE_FUNC>
- bool get_insert_lazy(Thread* thread, LOOKUP_FUNC& lookup_f, VALUE_FUNC& val_f,
- bool* grow_hint = NULL) {
- return get_insert_lazy(thread, lookup_f, val_f, noOp, grow_hint);
- }
-
- // Same without VALUE_FUNC.
- template <typename LOOKUP_FUNC, typename CALLBACK_FUNC>
- bool get_insert(Thread* thread, LOOKUP_FUNC& lookup_f, const VALUE& value,
- CALLBACK_FUNC& callback_f, bool* grow_hint = NULL) {
- LazyValueRetrieve vp(value);
- return get_insert_lazy(thread, lookup_f, vp, callback_f, grow_hint);
- }
-
- // Same without CALLBACK_FUNC and VALUE_FUNC.
- template <typename LOOKUP_FUNC>
- bool get_insert(Thread* thread, LOOKUP_FUNC& lookup_f, const VALUE& value,
- bool* grow_hint = NULL) {
- return get_insert(thread, lookup_f, value, noOp, grow_hint);
- }
-
// Get methods return true on found item with LOOKUP_FUNC and FOUND_FUNC is
// called.
template <typename LOOKUP_FUNC, typename FOUND_FUNC>
--- a/src/hotspot/share/utilities/concurrentHashTable.inline.hpp Thu Jan 10 16:50:26 2019 +0100
+++ b/src/hotspot/share/utilities/concurrentHashTable.inline.hpp Thu Jan 10 11:16:17 2019 -0600
@@ -874,78 +874,6 @@
}
template <typename VALUE, typename CONFIG, MEMFLAGS F>
-template <typename LOOKUP_FUNC, typename VALUE_FUNC, typename CALLBACK_FUNC>
-inline bool ConcurrentHashTable<VALUE, CONFIG, F>::
- 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;
- bool locked;
- size_t loops = 0;
- size_t i = 0;
- Node* new_node = NULL;
- uintx hash = lookup_f.get_hash();
- 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) {
- // No duplicate found.
- if (new_node == NULL) {
- new_node = Node::create_node(value_f(), first_at_start);
- } else {
- new_node->set_next(first_at_start);
- }
- if (bucket->cas_first(new_node, first_at_start)) {
- callback_f(true, new_node->value());
- 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.
- callback_f(false, old->value());
- 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 <typename VALUE, typename CONFIG, MEMFLAGS F>
template <typename LOOKUP_FUNC>
inline bool ConcurrentHashTable<VALUE, CONFIG, F>::
internal_insert(Thread* thread, LOOKUP_FUNC& lookup_f, const VALUE& value,
--- a/test/hotspot/gtest/utilities/test_concurrentHashtable.cpp Thu Jan 10 16:50:26 2019 +0100
+++ b/test/hotspot/gtest/utilities/test_concurrentHashtable.cpp Thu Jan 10 11:16:17 2019 -0600
@@ -81,39 +81,34 @@
delete cht;
}
-struct ValVerify {
- uintptr_t _val;
- bool called_get;
- bool called_insert;
- ValVerify(uintptr_t val) : _val(val), called_get(false), called_insert(false) {}
- void operator()(bool inserted, uintptr_t* val) {
- EXPECT_EQ(_val, *val) << "The value inserted is not correct.";
- if (inserted) {
- called_insert = true;
- } else {
- called_get = true;
- }
+struct ValueGet {
+ uintptr_t _return;
+ ValueGet() : _return(0) {}
+ void operator()(uintptr_t* value) {
+ EXPECT_NE(value, (uintptr_t*)NULL) << "expected valid value";
+ _return = *value;
}
- void verify(bool get, bool insert) {
- EXPECT_EQ(called_get, get) << "Get unexpected";
- EXPECT_EQ(called_insert, insert) << "Insert unexpected";
+ uintptr_t get_value() const {
+ return _return;
}
};
-static void cht_get_insert_helper(Thread* thr, SimpleTestTable* cht, uintptr_t val) {
+static void cht_get_helper(Thread* thr, SimpleTestTable* cht, uintptr_t val) {
{
SimpleTestLookup stl(val);
- ValVerify vv(val);
- EXPECT_EQ(cht->get_insert(thr, stl, val, vv), false) << "Inserting an unique value failed.";
- vv.verify(false, true);
+ ValueGet vg;
+ EXPECT_EQ(cht->get(thr, stl, vg), true) << "Getting an old value failed.";
+ EXPECT_EQ(val, vg.get_value()) << "Getting an old value failed.";
+ }
+}
+
+static void cht_insert_helper(Thread* thr, SimpleTestTable* cht, uintptr_t val) {
+ {
+ SimpleTestLookup stl(val);
+ EXPECT_EQ(cht->insert(thr, stl, val), true) << "Inserting an unique value failed.";
}
- {
- SimpleTestLookup stl(val);
- ValVerify vv(val);
- EXPECT_EQ(cht->get_insert(thr, stl, val, vv), true) << "Getting an old value failed.";
- vv.verify(true, false);
- }
+ cht_get_helper(thr, cht, val);
}
static void cht_get_insert(Thread* thr) {
@@ -123,7 +118,7 @@
{
SCOPED_TRACE("First");
- cht_get_insert_helper(thr, cht, val);
+ cht_insert_helper(thr, cht, val);
}
EXPECT_EQ(cht->get_copy(thr, stl), val) << "Get an old value failed";
EXPECT_TRUE(cht->remove(thr, stl)) << "Removing existing value failed.";
@@ -131,7 +126,7 @@
{
SCOPED_TRACE("Second");
- cht_get_insert_helper(thr, cht, val);
+ cht_insert_helper(thr, cht, val);
}
delete cht;
@@ -148,10 +143,13 @@
static void cht_getinsert_bulkdelete_insert_verified(Thread* thr, SimpleTestTable* cht, uintptr_t val,
bool verify_expect_get, bool verify_expect_inserted) {
- ValVerify vv(val);
SimpleTestLookup stl(val);
- EXPECT_EQ(cht->get_insert(thr, stl, val, vv), verify_expect_get) << "Inserting an unique value failed.";
- vv.verify(verify_expect_get, verify_expect_inserted);
+ if (verify_expect_inserted) {
+ cht_insert_helper(thr, cht, val);
+ }
+ if (verify_expect_get) {
+ cht_get_helper(thr, cht, val);
+ }
}
static void cht_getinsert_bulkdelete(Thread* thr) {
@@ -856,10 +854,19 @@
bool grow;
MyDel del(_br);
for (uintptr_t v = _start; v <= _stop; v++) {
- ValVerify vv(v);
- TestLookup tl(v);
- _cht->get_insert(this, tl, v, vv, &grow);
- EXPECT_NE(vv.called_get, vv.called_insert) << "Non or both callbacks was called.";
+ {
+ TestLookup tl(v);
+ ValueGet vg;
+ do {
+ if (_cht->get(this, tl, vg, &grow)) {
+ EXPECT_EQ(v, vg.get_value()) << "Getting an old value failed.";
+ break;
+ }
+ if (_cht->insert(this, tl, v, &grow)) {
+ break;
+ }
+ } while(true);
+ }
if (grow && !_shrink) {
_cht->grow(this);
}