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

Add Go benchmark test #3004

Merged
merged 1 commit into from Nov 16, 2021
Merged

Add Go benchmark test #3004

merged 1 commit into from Nov 16, 2021

Conversation

wenqiq
Copy link
Contributor

@wenqiq wenqiq commented Nov 8, 2021

Run the benchmarks once for each PR.

Fixes #2984

Signed-off-by: Wenqi Qiu wenqiq@vmware.com

@wenqiq
Copy link
Contributor Author

wenqiq commented Nov 8, 2021

See the benchmarks test duration:
https://github.com/wenqiq/antrea/runs/4143753174?check_suite_focus=true

@codecov-commenter
Copy link

codecov-commenter commented Nov 8, 2021

Codecov Report

Merging #3004 (c9a69eb) into main (873bd29) will increase coverage by 20.83%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##             main    #3004       +/-   ##
===========================================
+ Coverage   40.17%   61.01%   +20.83%     
===========================================
  Files         166      289      +123     
  Lines       20713    24548     +3835     
===========================================
+ Hits         8322    14978     +6656     
+ Misses      11580     7941     -3639     
- Partials      811     1629      +818     
Flag Coverage Δ
kind-e2e-tests 48.09% <ø> (?)
unit-tests 40.18% <ø> (+<0.01%) ⬆️

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

Impacted Files Coverage Δ
pkg/apiserver/handlers/webhook/convert_crd.go 2.56% <0.00%> (ø)
pkg/apiserver/handlers/webhook/mutation_labels.go 24.71% <0.00%> (ø)
...legacyclient/informers/externalversions/factory.go 56.00% <0.00%> (ø)
...clusterinformation/v1beta1/antreacontrollerinfo.go 0.00% <0.00%> (ø)
pkg/legacyapis/core/v1alpha2/webhook.go 0.00% <0.00%> (ø)
...et/versioned/typed/system/v1beta1/system_client.go 45.45% <0.00%> (ø)
pkg/apis/system/install/install.go 100.00% <0.00%> (ø)
pkg/agent/flowexporter/utils.go 71.42% <0.00%> (ø)
pkg/legacyapis/system/v1beta1/register.go 90.00% <0.00%> (ø)
...legacyclient/informers/externalversions/generic.go 0.00% <0.00%> (ø)
... and 230 more

- name: Check-out code
uses: actions/checkout@v2
- name: Run Go benchmark test
run: go test -run '^$' -bench . -benchtime 1s -timeout 10m -cpu 4 -v -benchmem ./pkg/...
Copy link
Contributor

Choose a reason for hiding this comment

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

why 1s instead of 1x? do you need to run each benchmark more than once? I think 1s is actually the default for -benchtime

Copy link
Contributor Author

@wenqiq wenqiq Nov 9, 2021

Choose a reason for hiding this comment

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

Yes, 1s is the default for -benchtime, 1x is more reasonable.

src/testing/benchmark.go

func initBenchmarkFlags() {
	matchBenchmarks = flag.String("test.bench", "", "run only benchmarks matching `regexp`")
	benchmarkMemory = flag.Bool("test.benchmem", false, "print memory allocations for benchmarks")
	flag.Var(&benchTime, "test.benchtime", "run each benchmark for duration `d`")
}

var (
	matchBenchmarks *string
	benchmarkMemory *bool

	benchTime = durationOrCountFlag{d: 1 * time.Second} // changed during test of testing package
)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated.

Run the benchmarks once for each PR.

Fixes antrea-io#2984

Signed-off-by: Wenqi Qiu <wenqiq@vmware.com>
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, but we need to merge #2994 first

Copy link
Member

@tnqn tnqn 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
Copy link
Member

tnqn commented Nov 16, 2021

/skip-all

@tnqn
Copy link
Member

tnqn commented Nov 16, 2021

/skip-e2e

@tnqn tnqn merged commit 363f2d8 into antrea-io:main Nov 16, 2021
jianjuns pushed a commit to jianjuns/antrea that referenced this pull request Dec 22, 2021
Run the benchmarks once for each PR.

Fixes antrea-io#2984

Signed-off-by: Wenqi Qiu <wenqiq@vmware.com>
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.

BenchmarkExportConntrackConns panics
4 participants