Performance improvement from brian-performance branch (#728)

* Prototype performance improvements

* Reverted AnyMap hash change

Signed-off-by: The MathWorks, Inc. <alchrist@mathworks.com>

* Fixed unused variable warnings

* Made changes requested by reviewers

* Made changes requested by reviewer

* Back out a performance improvement

Backing out a performance improvement as it creates a deadlock caused by mutex order locking.

Signed-off-by: The MathWorks, Inc. <jdicleme@mathworks.com>

Signed-off-by: The MathWorks, Inc. <alchrist@mathworks.com>
Signed-off-by: The MathWorks, Inc. <jdicleme@mathworks.com>
Co-authored-by: BrianWeed <brian_weed@yahoo.com>
Co-authored-by: jdicleme <jeffdiclemente@users.noreply.github.com>
Co-authored-by: Jeff <DiClemente>

compendium/DeclarativeServices/test/TestReferenceManagerImpl.cpp

@@ -611,7 +611,7 @@ namespace cppmicroservices
 
             auto registeredServiceRefs = bc.GetServiceReferences<dummy::Reference1>();
             auto registeredServiceCount = registeredServiceRefs.size();
-            // statuc-reluctant, mandatory-unary - depends on which thread's service was bound
+            // static-reluctant, mandatory-unary - depends on which thread's service was bound
             // static-reluctant, optional-unary - none of the services are bound
             if (refManager.IsOptional() && fakeMetadata.policyOption == ReferencePolicyOption_Reluctant
                 && !(ReferencePolicy_Dynamic == fakeMetadata.policy))
@@ -631,7 +631,6 @@ namespace cppmicroservices
             if (refManager.IsOptional())
             {
                 EXPECT_TRUE(refManager.IsSatisfied()) << "Ref should only be satisfied if it is optional";
-                ;
             }
             EXPECT_EQ(refManager.GetTargetReferences().size(), registeredServiceCount)
                 << "TargetReferences must be the same as any available services in the "

framework/src/service/ServiceListeners.cpp

@@ -478,7 +478,7 @@ namespace cppmicroservices
             }
 
             // Check the cache
-            auto const c = any_cast<std::vector<std::string>>(props->Value_unlocked(Constants::OBJECTCLASS).first);
+            auto const& c = ref_any_cast<std::vector<std::string>>(props->ValueByRef_unlocked(Constants::OBJECTCLASS));
             for (auto& objClass : c)
             {
                 AddToSet_unlocked(set, receivers, OBJECTCLASS_IX, objClass);

framework/src/service/ServiceReferenceBase.cpp

@@ -129,7 +129,9 @@ namespace cppmicroservices
     ServiceReferenceBase::operator<(ServiceReferenceBase const& reference) const
     {
         if (d.load() == reference.d.load())
+        {
             return false;
+        }
 
         if (!(*this))
         {
@@ -146,26 +148,31 @@ namespace cppmicroservices
             return false;
         }
 
+        /// A deadlock caused by mutex order locking will happen if these two scoped blocks
+        /// are combined into one. Multiple threads can enter this function as a result of
+        /// adding/removing ServiceReferenceBase objects from STL containers. If that occurs
+        /// AND these two scoped blocks are combined into one such that both "Lock()" calls
+        /// happen in the same scoped block, sporadic deadlocks will occur.
         Any anyR1;
         Any anyId1;
         {
-            auto l = d.load()->registration->properties.Lock();
-            US_UNUSED(l);
+            auto l1 = d.load()->registration->properties.Lock();
+            US_UNUSED(l1);
             anyR1 = d.load()->registration->properties.Value_unlocked(Constants::SERVICE_RANKING).first;
             assert(anyR1.Empty() || anyR1.Type() == typeid(int));
             anyId1 = d.load()->registration->properties.Value_unlocked(Constants::SERVICE_ID).first;
-            assert(anyId1.Type() == typeid(long int));
+            assert(anyId1.Empty() || anyId1.Type() == typeid(long int));
         }
 
         Any anyR2;
         Any anyId2;
         {
-            auto l = reference.d.load()->registration->properties.Lock();
-            US_UNUSED(l);
+            auto l2 = reference.d.load()->registration->properties.Lock();
+            US_UNUSED(l2);
             anyR2 = reference.d.load()->registration->properties.Value_unlocked(Constants::SERVICE_RANKING).first;
             assert(anyR2.Empty() || anyR2.Type() == typeid(int));
             anyId2 = reference.d.load()->registration->properties.Value_unlocked(Constants::SERVICE_ID).first;
-            assert(anyId2.Type() == typeid(long int));
+            assert(anyId2.Empty() || anyId2.Type() == typeid(long int));
         }
 
         int const r1 = anyR1.Empty() ? 0 : *any_cast<int>(&anyR1);

framework/src/service/ServiceRegistry.cpp

@@ -263,11 +263,9 @@ namespace cppmicroservices
 
         for (; s != send; ++s)
         {
-            ServiceReferenceBase sri = s->GetReference(clazz);
-
             if (filter.empty() || ldap.Evaluate(PropertiesHandle(s->d->properties, true), false))
             {
-                res.push_back(sri);
+                res.emplace_back(s->GetReference(clazz));
             }
         }
 

framework/src/util/Properties.cpp

@@ -103,6 +103,83 @@ namespace cppmicroservices
         return *this;
     }
 
+    Any const&
+    Properties::ValueByRef_unlocked(std::string const& key, bool matchCase) const
+    {
+        if (props.GetType() == AnyMap::UNORDERED_MAP_CASEINSENSITIVE_KEYS)
+        {
+            if (auto itr = props.findUOCI_TypeChecked(key); itr != props.endUOCI_TypeChecked())
+            {
+                if (!matchCase)
+                {
+                    return itr->second;
+                }
+                else if (matchCase && itr->first == key)
+                {
+                    return itr->second;
+                }
+                else
+                {
+                    return emptyAny;
+                }
+            }
+            else
+            {
+                return emptyAny;
+            }
+        }
+        else if (props.GetType() == AnyMap::UNORDERED_MAP)
+        {
+            auto itr = props.findUO_TypeChecked(key);
+            if (itr != props.endUO_TypeChecked())
+            {
+                return itr->second;
+            }
+
+            if (!matchCase)
+            {
+                auto ciItr = caseInsensitiveLookup.find(key);
+                if (ciItr != caseInsensitiveLookup.end())
+                {
+                    return props.findUO_TypeChecked(ciItr->second.data())->second;
+                }
+                else
+                {
+                    return emptyAny;
+                }
+            }
+
+            return emptyAny;
+        }
+        else if (props.GetType() == AnyMap::ORDERED_MAP)
+        {
+            auto itr = props.findOM_TypeChecked(key);
+            if (itr != props.endOM_TypeChecked())
+            {
+                return itr->second;
+            }
+
+            if (!matchCase)
+            {
+                auto ciItr = caseInsensitiveLookup.find(key);
+                if (ciItr != caseInsensitiveLookup.end())
+                {
+                    return props.findOM_TypeChecked(ciItr->second.data())->second;
+                }
+                else
+                {
+                    return emptyAny;
+                }
+            }
+
+            return emptyAny;
+        }
+        else
+        {
+            throw std::runtime_error("Unknown AnyMap type.");
+        }
+    }
+
     // This function has been modified to perform both the "find" and "lookup" operations rather than
     // just the "lookup" as originally written.
     //

framework/src/util/Properties.h

@@ -43,6 +43,8 @@ namespace cppmicroservices
         Properties(Properties&& o) noexcept;
         Properties& operator=(Properties&& o) noexcept;
 
+        Any const& ValueByRef_unlocked(std::string const& key, bool matchCase = false) const;
+
         std::pair<Any, bool> Value_unlocked(std::string const& key, bool matchCase = false) const;
 
         std::vector<std::string> Keys_unlocked() const;