Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve calculating symmetric difference #1944

Merged
merged 1 commit into from
Mar 10, 2021
Merged

Conversation

tnqn
Copy link
Member

@tnqn tnqn commented Mar 9, 2021

Calculating symmetric difference is common in the code, while
s1.Difference(s2).Union(s2.Difference(s1)) is a little complicated,
error-prone, and always builds several unnecessary intermediate sets.
This patch adds an util function SymmetricDifference to improve it. The
benchmark impact is as below when calculating symmetric difference of
two sets each of which contains 2000 items:

name                    old time/op    new time/op    delta
SymmetricDifference-48    1.50ms ± 1%    0.87ms ± 3%  -42.10%  (p=0.008 n=5+5)

name                    old alloc/op   new alloc/op   delta
SymmetricDifference-48     342kB ± 0%     171kB ± 0%  -49.99%  (p=0.008 n=5+5)

name                    old allocs/op  new allocs/op  delta
SymmetricDifference-48       132 ± 0%        61 ± 0%  -53.79%  (p=0.029 n=4+4)

Calculating symmetric difference is common in the code, while
s1.Difference(s2).Union(s2.Difference(s1)) is a little complicated,
error-prone, and always builds several unnecessary intermediate sets.
This patch adds an util function SymmetricDifference to improve it. The
benchmark impact is as below when calculating symmetric difference of
two sets each of which contains 2000 items:

name                    old time/op    new time/op    delta
SymmetricDifference-48    1.50ms ± 1%    0.87ms ± 3%  -42.10%  (p=0.008 n=5+5)

name                    old alloc/op   new alloc/op   delta
SymmetricDifference-48     342kB ± 0%     171kB ± 0%  -49.99%  (p=0.008 n=5+5)

name                    old allocs/op  new allocs/op  delta
SymmetricDifference-48       132 ± 0%        61 ± 0%  -53.79%  (p=0.029 n=4+4)
@codecov-io
Copy link

codecov-io commented Mar 9, 2021

Codecov Report

Merging #1944 (86dc4e2) into main (b7b0511) will increase coverage by 10.36%.
The diff coverage is 80.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #1944       +/-   ##
===========================================
+ Coverage   52.04%   62.41%   +10.36%     
===========================================
  Files         204      204               
  Lines       17527    17533        +6     
===========================================
+ Hits         9122    10943     +1821     
+ Misses       7203     5423     -1780     
+ Partials     1202     1167       -35     
Flag Coverage Δ
e2e-tests 23.23% <33.33%> (?)
kind-e2e-tests 51.27% <0.00%> (-0.78%) ⬇️
unit-tests 41.36% <60.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
pkg/agent/nodeportlocal/k8s/npl_controller.go 61.13% <0.00%> (ø)
...ntroller/networkpolicy/networkpolicy_controller.go 84.50% <60.00%> (+12.00%) ⬆️
pkg/controller/networkpolicy/external_entity.go 59.01% <100.00%> (+59.01%) ⬆️
pkg/util/sets/string.go 100.00% <100.00%> (ø)
pkg/antctl/transform/appliedtogroup/transform.go 0.00% <0.00%> (-43.48%) ⬇️
pkg/antctl/transform/utils.go 0.00% <0.00%> (-35.72%) ⬇️
pkg/agent/apiserver/handlers/agentinfo/handler.go 73.52% <0.00%> (-5.89%) ⬇️
pkg/agent/controller/traceflow/packetin.go 51.28% <0.00%> (-5.13%) ⬇️
...agent/controller/traceflow/traceflow_controller.go 71.93% <0.00%> (-3.17%) ⬇️
pkg/agent/route/route_linux.go 45.65% <0.00%> (-0.65%) ⬇️
... and 73 more

@tnqn
Copy link
Member Author

tnqn commented Mar 9, 2021

/test-all

@tnqn
Copy link
Member Author

tnqn commented Mar 9, 2021

/test-networkpolicy
/test-all-features-conformance

Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@tnqn tnqn merged commit d519002 into antrea-io:main Mar 10, 2021
@tnqn tnqn deleted the sets-diff branch March 10, 2021 02:30
tnqn added a commit to tnqn/antrea that referenced this pull request Mar 17, 2021
Calculating symmetric difference is common in the code, while
s1.Difference(s2).Union(s2.Difference(s1)) is a little complicated,
error-prone, and always builds several unnecessary intermediate sets.
This patch adds an util function SymmetricDifference to improve it. The
benchmark impact is as below when calculating symmetric difference of
two sets each of which contains 2000 items:

name                    old time/op    new time/op    delta
SymmetricDifference-48    1.50ms ± 1%    0.87ms ± 3%  -42.10%  (p=0.008 n=5+5)

name                    old alloc/op   new alloc/op   delta
SymmetricDifference-48     342kB ± 0%     171kB ± 0%  -49.99%  (p=0.008 n=5+5)

name                    old allocs/op  new allocs/op  delta
SymmetricDifference-48       132 ± 0%        61 ± 0%  -53.79%  (p=0.029 n=4+4)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants