Use GroupMemberSet.Merge to reduce CPU usage and memory footprint #2467
Conversation
Codecov Report
@@ Coverage Diff @@
## main #2467 +/- ##
==========================================
+ Coverage 59.82% 64.97% +5.14%
==========================================
Files 284 284
Lines 22168 26080 +3912
==========================================
+ Hits 13263 16946 +3683
- Misses 7483 7547 +64
- Partials 1422 1587 +165
Flags with carried forward coverage won't be shown. Click here to find out more.
|
pkg/apis/controlplane/sets.go
Outdated
| // s1.Merge(s2) = {a1, a2, a3, a4, a5} | ||
| // s1 = {a1, a2, a3, a4, a5} | ||
| // | ||
| // It supersedes s1.Union(s2) when constructing a new set is not the intention. |
There was a problem hiding this comment.
| // It supersedes s1.Union(s2) when constructing a new set is not the intention. | |
| // It should be used instead of s1.Union(s2) when constructing a new set is not required. |
same below
Union and merge mean different things IMO, so we shouldn't just change the implementation of union |
Yes, also Union is the original method of K8s util package. It's widely used in the scenario where constructing a new set is required. |
|
/test-all |
When getting the union of multiple AddressGroups or AppliedToGroups, agent used the GroupMemberSet.Union method, which always creates an intermediate set and copies the original items when merging another set. This wasted significant time and memory when the groups had massive members. This patch adds the GroupMemberSet.Merge method which avoids the above overhead. benchmark comparison when merging two groups with 10K members and 100 members: name old time/op new time/op delta RuleCacheUnionAddressGroups-48 4.27ms ±15% 2.15ms ±15% -49.72% (p=0.008 n=5+5) name old alloc/op new alloc/op delta RuleCacheUnionAddressGroups-48 1.95MB ± 0% 0.97MB ± 0% -50.00% (p=0.008 n=5+5) name old allocs/op new allocs/op delta RuleCacheUnionAddressGroups-48 565 ± 0% 282 ± 0% -50.16% (p=0.000 n=4+5) Signed-off-by: Quan Tian <qtian@vmware.com>
tnqn
force-pushed
the
improve-union-group
branch
from
1045e89
to
5856d64
Compare
Thanks for the explanations. LGTM |
|
/test-integration |
When getting the union of multiple AddressGroups or AppliedToGroups,
agent used the GroupMemberSet.Union method, which always creates an
intermediate set and copies the original items when merging another
set. This wasted significant time and memory when the groups had
massive members. This patch adds the GroupMemberSet.Merge method which
avoids the above overhead.
benchmark comparison when merging two groups with 10K members and 100
members:
Signed-off-by: Quan Tian qtian@vmware.com
For #2457