Enable EndpointIterator to target search single group ID (#25758)

* Enable EndpointIterator to target search single group ID Running RR_1_1 test where we stress test by using max of everything on every fabric, it was found that each iterator step could take up to 21ms. For a large enough group table ReadGroupTable was taking more then 5 second. By targeting the group id for which we want to iterator over endpoints we can reduce time `ReadGroupTable` for large tables by an order of magnitude. * Added comment * Apply suggestions from code review Co-authored-by: Boris Zbarsky <[email protected]> * Restyle --------- Co-authored-by: Boris Zbarsky <[email protected]>
project-chip · Jan 22, 2024 · 4849379 · 4849379
1 parent fa75259
commit 4849379
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 19 deletions.
diff --git a/src/app/clusters/group-key-mgmt-server/group-key-mgmt-server.cpp b/src/app/clusters/group-key-mgmt-server/group-key-mgmt-server.cpp
@@ -84,15 +84,12 @@ struct GroupTableCodec
         TLV::TLVType inner;
         ReturnErrorOnFailure(writer.StartContainer(TagEndpoints(), TLV::kTLVType_Array, inner));
         GroupDataProvider::GroupEndpoint mapping;
-        auto iter = mProvider->IterateEndpoints(mFabric);
+        auto iter = mProvider->IterateEndpoints(mFabric, MakeOptional(mInfo.group_id));
         if (nullptr != iter)
         {
             while (iter->Next(mapping))
             {
-                if (mapping.group_id == mInfo.group_id)
-                {
-                    ReturnErrorOnFailure(writer.Put(TLV::AnonymousTag(), static_cast<uint16_t>(mapping.endpoint_id)));
-                }
+                ReturnErrorOnFailure(writer.Put(TLV::AnonymousTag(), static_cast<uint16_t>(mapping.endpoint_id)));
             }
             iter->Release();
         }

diff --git a/src/credentials/GroupDataProvider.h b/src/credentials/GroupDataProvider.h
@@ -249,10 +249,11 @@ class GroupDataProvider
      *  Creates an iterator that may be used to obtain the list of (group, endpoint) pairs associated with the given fabric.
      *  In order to release the allocated memory, the Release() method must be called after the iteration is finished.
      *  Modifying the group table during the iteration is currently not supported, and may yield unexpected behaviour.
+     *  If you wish to iterate only the endpoints of a particular group id you can provide the optional `group_id` to do so.
      *  @retval An instance of EndpointIterator on success
      *  @retval nullptr if no iterator instances are available.
      */
-    virtual EndpointIterator * IterateEndpoints(FabricIndex fabric_index) = 0;
+    virtual EndpointIterator * IterateEndpoints(FabricIndex fabric_index, Optional<GroupId> group_id = NullOptional) = 0;
 
     //
     // Group-Key map

diff --git a/src/credentials/GroupDataProviderImpl.cpp b/src/credentials/GroupDataProviderImpl.cpp
@@ -1207,28 +1207,43 @@ void GroupDataProviderImpl::GroupInfoIteratorImpl::Release()
     mProvider.mGroupInfoIterators.ReleaseObject(this);
 }
 
-GroupDataProvider::EndpointIterator * GroupDataProviderImpl::IterateEndpoints(chip::FabricIndex fabric_index)
+GroupDataProvider::EndpointIterator * GroupDataProviderImpl::IterateEndpoints(chip::FabricIndex fabric_index,
+                                                                              Optional<GroupId> group_id)
 {
     VerifyOrReturnError(IsInitialized(), nullptr);
-    return mEndpointIterators.CreateObject(*this, fabric_index);
+    return mEndpointIterators.CreateObject(*this, fabric_index, group_id);
 }
 
-GroupDataProviderImpl::EndpointIteratorImpl::EndpointIteratorImpl(GroupDataProviderImpl & provider,
-                                                                  chip::FabricIndex fabric_index) :
+GroupDataProviderImpl::EndpointIteratorImpl::EndpointIteratorImpl(GroupDataProviderImpl & provider, chip::FabricIndex fabric_index,
+                                                                  Optional<GroupId> group_id) :
     mProvider(provider),
     mFabric(fabric_index)
 {
     FabricData fabric(fabric_index);
     VerifyOrReturn(CHIP_NO_ERROR == fabric.Load(provider.mStorage));
 
-    GroupData group(fabric_index, fabric.first_group);
-    VerifyOrReturn(CHIP_NO_ERROR == group.Load(provider.mStorage));
+    if (group_id.HasValue())
+    {
+        GroupData group(fabric_index, group_id.Value());
+        VerifyOrReturn(CHIP_NO_ERROR == group.Load(provider.mStorage));
+
+        mGroup         = group_id.Value();
+        mFirstGroup    = group_id.Value();
+        mGroupCount    = 1;
+        mEndpoint      = group.first_endpoint;
+        mEndpointCount = group.endpoint_count;
+    }
+    else
+    {
+        GroupData group(fabric_index, fabric.first_group);
+        VerifyOrReturn(CHIP_NO_ERROR == group.Load(provider.mStorage));
 
-    mGroup         = fabric.first_group;
-    mFirstGroup    = fabric.first_group;
-    mGroupCount    = fabric.group_count;
-    mEndpoint      = group.first_endpoint;
-    mEndpointCount = group.endpoint_count;
+        mGroup         = fabric.first_group;
+        mFirstGroup    = fabric.first_group;
+        mGroupCount    = fabric.group_count;
+        mEndpoint      = group.first_endpoint;
+        mEndpointCount = group.endpoint_count;
+    }
 }
 
 size_t GroupDataProviderImpl::EndpointIteratorImpl::Count()

diff --git a/src/credentials/GroupDataProviderImpl.h b/src/credentials/GroupDataProviderImpl.h
@@ -68,7 +68,7 @@ class GroupDataProviderImpl : public GroupDataProvider
     CHIP_ERROR RemoveEndpoint(FabricIndex fabric_index, EndpointId endpoint_id) override;
     // Iterators
     GroupInfoIterator * IterateGroupInfo(FabricIndex fabric_index) override;
-    EndpointIterator * IterateEndpoints(FabricIndex fabric_index) override;
+    EndpointIterator * IterateEndpoints(FabricIndex fabric_index, Optional<GroupId> group_id = NullOptional) override;
 
     //
     // Group-Key map
@@ -133,7 +133,7 @@ class GroupDataProviderImpl : public GroupDataProvider
     class EndpointIteratorImpl : public EndpointIterator
     {
     public:
-        EndpointIteratorImpl(GroupDataProviderImpl & provider, FabricIndex fabric_index);
+        EndpointIteratorImpl(GroupDataProviderImpl & provider, FabricIndex fabric_index, Optional<GroupId> group_id);
         size_t Count() override;
         bool Next(GroupEndpoint & output) override;
         void Release() override;