You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@geode.apache.org by bb...@apache.org on 2020/06/22 20:54:43 UTC
[geode-native] branch develop updated: GEODE-8212: Reduce
connections to server to get type id (#619)
This is an automated email from the ASF dual-hosted git repository.
bbender pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/geode-native.git
The following commit(s) were added to refs/heads/develop by this push:
new d472d12 GEODE-8212: Reduce connections to server to get type id (#619)
d472d12 is described below
commit d472d12bb9d924466d67b9da1535430525a01bef
Author: Alberto Bustamante Reyes <al...@users.noreply.github.com>
AuthorDate: Mon Jun 22 22:54:36 2020 +0200
GEODE-8212: Reduce connections to server to get type id (#619)
- change the type of pdxTypeToTypeIdMap to unordered_map and use a custom hash function and aequality comparator based on that hash function.
- hash field types and names to generate our own local ID for map key. Look up based on that key before tripping to server to get a real type ID
---
cppcache/benchmark/CMakeLists.txt | 1 +
cppcache/integration/benchmark/CMakeLists.txt | 2 +-
cppcache/integration/benchmark/PdxTypeBM.cpp | 41 ++++++
cppcache/src/PdxFieldType.cpp | 16 ++
cppcache/src/PdxFieldType.hpp | 4 +
cppcache/src/PdxHelper.cpp | 6 +-
cppcache/src/PdxType.cpp | 35 +++--
cppcache/src/PdxType.hpp | 27 +++-
cppcache/src/PdxTypeRegistry.cpp | 2 -
cppcache/src/PdxTypeRegistry.hpp | 13 +-
cppcache/test/CMakeLists.txt | 4 +-
cppcache/test/PdxTypeTest.cpp | 201 ++++++++++++++++++++++++++
12 files changed, 314 insertions(+), 38 deletions(-)
diff --git a/cppcache/benchmark/CMakeLists.txt b/cppcache/benchmark/CMakeLists.txt
index cdcfe34..00bcb8c 100644
--- a/cppcache/benchmark/CMakeLists.txt
+++ b/cppcache/benchmark/CMakeLists.txt
@@ -25,6 +25,7 @@ target_link_libraries(cpp-benchmark
PUBLIC
apache-geode-static
benchmark::benchmark
+ integration-framework
PRIVATE
_WarningsAsError
)
diff --git a/cppcache/integration/benchmark/CMakeLists.txt b/cppcache/integration/benchmark/CMakeLists.txt
index c15ffeb..88254d2 100644
--- a/cppcache/integration/benchmark/CMakeLists.txt
+++ b/cppcache/integration/benchmark/CMakeLists.txt
@@ -16,7 +16,7 @@
add_executable(cpp-integration-benchmark
main.cpp
RegionBM.cpp
- )
+ PdxTypeBM.cpp)
target_link_libraries(cpp-integration-benchmark
PUBLIC
diff --git a/cppcache/integration/benchmark/PdxTypeBM.cpp b/cppcache/integration/benchmark/PdxTypeBM.cpp
new file mode 100644
index 0000000..0b06ee1
--- /dev/null
+++ b/cppcache/integration/benchmark/PdxTypeBM.cpp
@@ -0,0 +1,41 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+
+#include <benchmark/benchmark.h>
+#include <framework/Cluster.h>
+
+static void PdxTypeBM_createInstance(benchmark::State& state) {
+ const std::string gemfireJsonClassName = "__GEMFIRE_JSON";
+
+ Cluster cluster{Name("PdxTypeBM"), LocatorCount{1}, ServerCount{1}};
+ cluster.start();
+ cluster.getGfsh().create();
+
+ auto cache = cluster.createCache();
+ auto repetitions = 1000;
+
+ for (auto _ : state) {
+ for (auto i = 0; i < repetitions; ++i) {
+ auto instance_factory =
+ cache.createPdxInstanceFactory(gemfireJsonClassName);
+ instance_factory.writeString("bar", std::to_string(i));
+ auto instance = instance_factory.create();
+ }
+ }
+}
+
+BENCHMARK(PdxTypeBM_createInstance)->UseRealTime();
diff --git a/cppcache/src/PdxFieldType.cpp b/cppcache/src/PdxFieldType.cpp
index 360ed61..b5b5bd8 100644
--- a/cppcache/src/PdxFieldType.cpp
+++ b/cppcache/src/PdxFieldType.cpp
@@ -139,6 +139,22 @@ std::string PdxFieldType::toString() const {
return std::string(stringBuf);
}
+bool PdxFieldType::operator==(const PdxFieldType& other) const {
+ if (m_className != other.m_className) {
+ return false;
+ }
+
+ if (m_fieldName != other.m_fieldName) {
+ return false;
+ }
+
+ return true;
+}
+
+bool PdxFieldType::operator!=(const PdxFieldType& other) const {
+ return !(*this == other);
+}
+
} // namespace client
} // namespace geode
} // namespace apache
diff --git a/cppcache/src/PdxFieldType.hpp b/cppcache/src/PdxFieldType.hpp
index ea04687..ffd24bf 100644
--- a/cppcache/src/PdxFieldType.hpp
+++ b/cppcache/src/PdxFieldType.hpp
@@ -105,6 +105,10 @@ class APACHE_GEODE_EXPORT PdxFieldType
int32_t getVarLenOffsetIndex() const { return m_vlOffsetIndex; }
int32_t getRelativeOffset() const { return m_relativeOffset; }
+
+ bool operator==(const PdxFieldType& other) const;
+
+ bool operator!=(const PdxFieldType& other) const;
};
} // namespace client
diff --git a/cppcache/src/PdxHelper.cpp b/cppcache/src/PdxHelper.cpp
index 806b8d3..fc2f97a 100644
--- a/cppcache/src/PdxHelper.cpp
+++ b/cppcache/src/PdxHelper.cpp
@@ -214,7 +214,7 @@ std::shared_ptr<PdxSerializable> PdxHelper::deserializePdx(DataInput& dataInput,
// Check for the PdxWrapper
pdxLocalType = prtc.getLocalType();
- if (pType->Equals(pdxLocalType)) {
+ if (*pType == *pdxLocalType) {
pdxTypeRegistry->addLocalPdxType(pdxRealObject->getClassName(), pType);
pdxTypeRegistry->addPdxType(pType->getTypeId(), pType);
pType->setLocal(true);
@@ -325,9 +325,9 @@ void PdxHelper::createMergedType(std::shared_ptr<PdxType> localType,
auto pdxTypeRegistry = cacheImpl->getPdxTypeRegistry();
auto serializaionRegistry = cacheImpl->getSerializationRegistry();
- if (mergedVersion->Equals(localType)) {
+ if (*mergedVersion == *localType) {
pdxTypeRegistry->setMergedType(remoteType->getTypeId(), localType);
- } else if (mergedVersion->Equals(remoteType)) {
+ } else if (*mergedVersion == *remoteType) {
pdxTypeRegistry->setMergedType(remoteType->getTypeId(), remoteType);
} else { // need to create new version
mergedVersion->InitializeType();
diff --git a/cppcache/src/PdxType.cpp b/cppcache/src/PdxType.cpp
index 348acf4..b4d66de 100644
--- a/cppcache/src/PdxType.cpp
+++ b/cppcache/src/PdxType.cpp
@@ -537,33 +537,32 @@ void PdxType::generatePositionMap() {
}
}
-bool PdxType::Equals(std::shared_ptr<PdxType> otherObj) {
- if (otherObj == nullptr) return false;
-
- PdxType* ot = dynamic_cast<PdxType*>(otherObj.get());
-
- if (ot == nullptr) return false;
+bool PdxType::operator==(const PdxType& other) const {
+ if (this->m_className != other.m_className) {
+ return false;
+ }
- if (ot == this) return true;
+ if (this->m_noJavaClass != other.m_noJavaClass) {
+ return false;
+ }
- if (ot->m_pdxFieldTypes->size() != m_pdxFieldTypes->size()) return false;
+ if (this->getTotalFields() != other.getTotalFields()) {
+ return false;
+ }
- for (uint32_t i = 0; i < m_pdxFieldTypes->size(); i++) {
- if (!ot->m_pdxFieldTypes->at(i)->equals(m_pdxFieldTypes->at(i))) {
+ auto thisIt = this->m_fieldNameVsPdxType.begin();
+ auto otherIt = other.m_fieldNameVsPdxType.begin();
+ for (; thisIt != this->m_fieldNameVsPdxType.end(); thisIt++, otherIt++) {
+ auto thisEntry = *thisIt;
+ auto otherEntry = *otherIt;
+ if (thisEntry.first != otherEntry.first ||
+ *(thisEntry.second) != *(otherEntry.second)) {
return false;
}
}
return true;
}
-bool PdxType::operator<(const PdxType& other) const {
- auto typeIdLessThan = this->m_geodeTypeId < other.m_geodeTypeId;
- auto typeIdsBothZero =
- (this->m_geodeTypeId == 0) && (other.m_geodeTypeId == 0);
- auto classnameLessThan = this->m_className < other.m_className;
- return (typeIdLessThan || (typeIdsBothZero && classnameLessThan));
-}
-
} // namespace client
} // namespace geode
} // namespace apache
diff --git a/cppcache/src/PdxType.hpp b/cppcache/src/PdxType.hpp
index 3995d28..26ac19c 100644
--- a/cppcache/src/PdxType.hpp
+++ b/cppcache/src/PdxType.hpp
@@ -200,13 +200,34 @@ class PdxType : public internal::DataSerializableInternal,
int32_t* getRemoteToLocalMap();
- bool Equals(std::shared_ptr<PdxType> otherObj);
+ bool operator==(const PdxType& other) const;
- // This is for PdxType as key in std map.
- bool operator<(const PdxType& other) const;
+ NameVsPdxType getFieldNameVsPdxType() const { return m_fieldNameVsPdxType; }
};
} // namespace client
} // namespace geode
} // namespace apache
+namespace std {
+
+template <>
+struct hash<apache::geode::client::PdxType> {
+ typedef apache::geode::client::PdxType argument_type;
+ typedef size_t result_type;
+ result_type operator()(const argument_type& val) const {
+ std::hash<std::string> strHash;
+ auto result = strHash(val.getPdxClassName());
+
+ for (auto entry : val.getFieldNameVsPdxType()) {
+ auto pdxPtr = entry.second;
+ result = result ^ (strHash(pdxPtr->getClassName()) << 1);
+ result = result ^ (strHash(pdxPtr->getFieldName()) << 1);
+ }
+
+ return result;
+ }
+};
+
+} // namespace std
+
#endif // GEODE_PDXTYPE_H_
diff --git a/cppcache/src/PdxTypeRegistry.cpp b/cppcache/src/PdxTypeRegistry.cpp
index 9c520da..7fc17a6 100644
--- a/cppcache/src/PdxTypeRegistry.cpp
+++ b/cppcache/src/PdxTypeRegistry.cpp
@@ -79,7 +79,6 @@ int32_t PdxTypeRegistry::getPDXIdForType(std::shared_ptr<PdxType> nType,
{
WriteGuard write(g_readerWriterLock);
-
auto&& iter = pdxTypeToTypeIdMap.find(nType);
if (iter != pdxTypeToTypeIdMap.end()) {
typeId = iter->second;
@@ -92,7 +91,6 @@ int32_t PdxTypeRegistry::getPDXIdForType(std::shared_ptr<PdxType> nType,
nType->setTypeId(typeId);
pdxTypeToTypeIdMap.insert(std::make_pair(nType, typeId));
}
-
addPdxType(typeId, nType);
return typeId;
}
diff --git a/cppcache/src/PdxTypeRegistry.hpp b/cppcache/src/PdxTypeRegistry.hpp
index 8b266e5..70f4cdd 100644
--- a/cppcache/src/PdxTypeRegistry.hpp
+++ b/cppcache/src/PdxTypeRegistry.hpp
@@ -38,14 +38,6 @@ namespace apache {
namespace geode {
namespace client {
-struct PdxTypeLessThan {
- bool operator()(std::shared_ptr<PdxType> const& n1,
- std::shared_ptr<PdxType> const& n2) const {
- // call to PdxType::operator <()
- return *n1 < *n2;
- }
-};
-
typedef std::map<int32_t, std::shared_ptr<PdxType>> TypeIdVsPdxType;
typedef std::map<std::string, std::shared_ptr<PdxType>> TypeNameVsPdxType;
typedef std::unordered_map<std::shared_ptr<PdxSerializable>,
@@ -53,7 +45,10 @@ typedef std::unordered_map<std::shared_ptr<PdxSerializable>,
dereference_hash<std::shared_ptr<CacheableKey>>,
dereference_equal_to<std::shared_ptr<CacheableKey>>>
PreservedHashMap;
-typedef std::map<std::shared_ptr<PdxType>, int32_t, PdxTypeLessThan>
+
+typedef std::unordered_map<std::shared_ptr<PdxType>, int32_t,
+ dereference_hash<std::shared_ptr<PdxType>>,
+ dereference_equal_to<std::shared_ptr<PdxType>>>
PdxTypeToTypeIdMap;
class APACHE_GEODE_EXPORT PdxTypeRegistry
diff --git a/cppcache/test/CMakeLists.txt b/cppcache/test/CMakeLists.txt
index ef7c193..4fc52f1 100644
--- a/cppcache/test/CMakeLists.txt
+++ b/cppcache/test/CMakeLists.txt
@@ -42,6 +42,7 @@ add_executable(apache-geode_unittests
InterestResultPolicyTest.cpp
LocalRegionTest.cpp
PdxInstanceImplTest.cpp
+ PdxTypeTest.cpp
QueueConnectionRequestTest.cpp
RegionAttributesFactoryTest.cpp
SerializableCreateTests.cpp
@@ -55,8 +56,7 @@ add_executable(apache-geode_unittests
util/synchronized_mapTest.cpp
util/synchronized_setTest.cpp
util/TestableRecursiveMutex.hpp
- util/chrono/durationTest.cpp
-)
+ util/chrono/durationTest.cpp)
target_compile_definitions(apache-geode_unittests
PUBLIC
diff --git a/cppcache/test/PdxTypeTest.cpp b/cppcache/test/PdxTypeTest.cpp
new file mode 100644
index 0000000..938e061
--- /dev/null
+++ b/cppcache/test/PdxTypeTest.cpp
@@ -0,0 +1,201 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.
+ */
+
+#include <gtest/gtest.h>
+
+#include "PdxType.hpp"
+#include "PdxTypeRegistry.hpp"
+#include "PdxTypes.hpp"
+
+namespace {
+using apache::geode::client::PdxFieldTypes;
+using apache::geode::client::PdxTypeRegistry;
+using apache::geode::client::PdxTypes;
+
+const std::string gemfireJsonClassName = "__GEMFIRE_JSON";
+
+TEST(PdxTypeTest, testTwoObjectsWithSameClassnameAndSameFieldsAreEquals) {
+ PdxTypeRegistry pdxTypeRegistry(nullptr);
+
+ apache::geode::client::PdxType m_pdxType1(pdxTypeRegistry,
+ gemfireJsonClassName, false);
+ apache::geode::client::PdxType m_pdxType2(pdxTypeRegistry,
+ gemfireJsonClassName, false);
+
+ m_pdxType1.addVariableLengthTypeField("bar0", "string",
+ PdxFieldTypes::STRING);
+ m_pdxType1.addVariableLengthTypeField("bar1", "string",
+ PdxFieldTypes::STRING);
+ m_pdxType2.addVariableLengthTypeField("bar0", "string",
+ PdxFieldTypes::STRING);
+ m_pdxType2.addVariableLengthTypeField("bar1", "string",
+ PdxFieldTypes::STRING);
+
+ EXPECT_TRUE(m_pdxType1 == m_pdxType2);
+}
+
+TEST(PdxTypeTest,
+ testTwoObjectsWithSameClassnameAndSameFieldsInDifferentOrderAreEquals) {
+ PdxTypeRegistry pdxTypeRegistry(nullptr);
+
+ apache::geode::client::PdxType m_pdxType1(pdxTypeRegistry,
+ gemfireJsonClassName, false);
+ apache::geode::client::PdxType m_pdxType2(pdxTypeRegistry,
+ gemfireJsonClassName, false);
+ m_pdxType1.addVariableLengthTypeField("bar1", "string",
+ PdxFieldTypes::STRING);
+ m_pdxType1.addVariableLengthTypeField("bar0", "string",
+ PdxFieldTypes::STRING);
+ m_pdxType2.addVariableLengthTypeField("bar0", "string",
+ PdxFieldTypes::STRING);
+ m_pdxType2.addVariableLengthTypeField("bar1", "string",
+ PdxFieldTypes::STRING);
+
+ EXPECT_TRUE(m_pdxType1 == m_pdxType2);
+}
+
+TEST(PdxTypeTest,
+ testTwoObjectsWithDifferentClassnameButSameFieldsAreNotEquals) {
+ PdxTypeRegistry pdxTypeRegistry(nullptr);
+
+ apache::geode::client::PdxType m_pdxType1(pdxTypeRegistry,
+ gemfireJsonClassName, false);
+ apache::geode::client::PdxType m_pdxType2(pdxTypeRegistry, "otherClassName",
+ false);
+
+ m_pdxType1.addVariableLengthTypeField("bar0", "string",
+ PdxFieldTypes::STRING);
+ m_pdxType1.addVariableLengthTypeField("bar1", "string",
+ PdxFieldTypes::STRING);
+ m_pdxType2.addVariableLengthTypeField("bar0", "string",
+ PdxFieldTypes::STRING);
+ m_pdxType2.addVariableLengthTypeField("bar1", "string",
+ PdxFieldTypes::STRING);
+
+ EXPECT_FALSE(m_pdxType1 == m_pdxType2);
+}
+
+TEST(PdxTypeTest, testTwoObjectsWithSameFieldsHaveTheSameHash) {
+ PdxTypeRegistry pdxTypeRegistry(nullptr);
+
+ apache::geode::client::PdxType m_pdxType1(pdxTypeRegistry,
+ gemfireJsonClassName, false);
+ apache::geode::client::PdxType m_pdxType2(pdxTypeRegistry,
+ gemfireJsonClassName, false);
+
+ m_pdxType1.addVariableLengthTypeField("bar0", "string",
+ PdxFieldTypes::STRING);
+ m_pdxType1.addVariableLengthTypeField("bar1", "string",
+ PdxFieldTypes::STRING);
+ m_pdxType1.addVariableLengthTypeField("bar2", "string",
+ PdxFieldTypes::STRING);
+
+ m_pdxType2.addVariableLengthTypeField("bar0", "string",
+ PdxFieldTypes::STRING);
+ m_pdxType2.addVariableLengthTypeField("bar1", "string",
+ PdxFieldTypes::STRING);
+ m_pdxType2.addVariableLengthTypeField("bar2", "string",
+ PdxFieldTypes::STRING);
+
+ std::hash<apache::geode::client::PdxType> type1Hash;
+ std::hash<apache::geode::client::PdxType> type2Hash;
+
+ EXPECT_EQ(type1Hash(m_pdxType1), type2Hash(m_pdxType2));
+}
+
+TEST(PdxTypeTest, testTwoObjectsWithDifferentFieldsHaveDifferentHash) {
+ PdxTypeRegistry pdxTypeRegistry(nullptr);
+
+ apache::geode::client::PdxType m_pdxType1(pdxTypeRegistry,
+ gemfireJsonClassName, false);
+ apache::geode::client::PdxType m_pdxType2(pdxTypeRegistry,
+ gemfireJsonClassName, false);
+
+ m_pdxType1.addVariableLengthTypeField("bar0", "string",
+ PdxFieldTypes::STRING);
+ m_pdxType1.addVariableLengthTypeField("bar1", "string",
+ PdxFieldTypes::STRING);
+
+ m_pdxType2.addVariableLengthTypeField("bar2", "string",
+ PdxFieldTypes::STRING);
+ m_pdxType2.addVariableLengthTypeField("bar3", "string",
+ PdxFieldTypes::STRING);
+
+ std::hash<apache::geode::client::PdxType> type1Hash;
+ std::hash<apache::geode::client::PdxType> type2Hash;
+
+ EXPECT_NE(type1Hash(m_pdxType1), type2Hash(m_pdxType2));
+}
+
+TEST(PdxTypeTest, testTwoObjectsWithSameFieldsInDifferentOrderHaveTheSameHash) {
+ PdxTypeRegistry pdxTypeRegistry(nullptr);
+
+ apache::geode::client::PdxType m_pdxType1(pdxTypeRegistry,
+ gemfireJsonClassName, false);
+ apache::geode::client::PdxType m_pdxType2(pdxTypeRegistry,
+ gemfireJsonClassName, false);
+
+ m_pdxType1.addVariableLengthTypeField("bar0", "string",
+ PdxFieldTypes::STRING);
+ m_pdxType1.addVariableLengthTypeField("bar1", "string",
+ PdxFieldTypes::STRING);
+ m_pdxType1.addVariableLengthTypeField("bar2", "string",
+ PdxFieldTypes::STRING);
+
+ m_pdxType2.addVariableLengthTypeField("bar1", "string",
+ PdxFieldTypes::STRING);
+ m_pdxType2.addVariableLengthTypeField("bar2", "string",
+ PdxFieldTypes::STRING);
+ m_pdxType2.addVariableLengthTypeField("bar0", "string",
+ PdxFieldTypes::STRING);
+
+ std::hash<apache::geode::client::PdxType> type1Hash;
+ std::hash<apache::geode::client::PdxType> type2Hash;
+
+ EXPECT_EQ(type1Hash(m_pdxType1), type2Hash(m_pdxType2));
+}
+
+TEST(PdxTypeTest,
+ testTwoObjectsWithSameFieldsNamesButDifferentTypesHaveDifferentHash) {
+ PdxTypeRegistry pdxTypeRegistry(nullptr);
+
+ apache::geode::client::PdxType m_pdxType1(pdxTypeRegistry,
+ gemfireJsonClassName, false);
+ apache::geode::client::PdxType m_pdxType2(pdxTypeRegistry,
+ gemfireJsonClassName, false);
+
+ m_pdxType1.addVariableLengthTypeField("bar0", "string",
+ PdxFieldTypes::STRING);
+ m_pdxType1.addVariableLengthTypeField("bar1", "string",
+ PdxFieldTypes::STRING);
+ m_pdxType1.addVariableLengthTypeField("bar2", "string",
+ PdxFieldTypes::STRING);
+
+ m_pdxType2.addVariableLengthTypeField("bar0", "string",
+ PdxFieldTypes::STRING);
+ m_pdxType2.addFixedLengthTypeField("bar1", "bool", PdxFieldTypes::BOOLEAN,
+ PdxTypes::BOOLEAN_SIZE);
+ m_pdxType2.addFixedLengthTypeField("bar2", "int", PdxFieldTypes::INT,
+ PdxTypes::INTEGER_SIZE);
+
+ std::hash<apache::geode::client::PdxType> type1Hash;
+ std::hash<apache::geode::client::PdxType> type2Hash;
+
+ EXPECT_NE(type1Hash(m_pdxType1), type2Hash(m_pdxType2));
+}
+
+} // namespace