[Refactor] Improve errors.Join performance #2010

kpango · 2023-04-19T06:08:16Z

Description:

Since errors.Is was used to evaluate the equality of errors in errors.Join for the 0th and subsequent errors, performance problems occurred exponentially depending on the length of the errors.

By eliminating the evaluation of errors.Is, I have confirmed that performance can be improved.

❯ go test -count=3 -timeout=1h -run=NONE -bench . -benchmem
goos: linux
goarch: amd64
pkg: github.com/vdaas/vald/internal/errors
cpu: AMD Ryzen Threadripper 3990X 64-Core Processor
BenchmarkWrapLongData-128                     30          34398736 ns/op        138625268 B/op     88049 allocs/op
BenchmarkWrapLongData-128                     57          19898078 ns/op        137000936 B/op     47636 allocs/op
BenchmarkWrapLongData-128                     20          50354703 ns/op        140355950 B/op    130721 allocs/op
BenchmarkWrapShortData-128                 61291             16516 ns/op           10668 B/op         53 allocs/op
BenchmarkWrapShortData-128                 54051             19190 ns/op           10345 B/op         59 allocs/op
BenchmarkWrapShortData-128                142771              7767 ns/op           13588 B/op         30 allocs/op
BenchmarkJoinLongData-128                  36448             27727 ns/op           40963 B/op         71 allocs/op
BenchmarkJoinLongData-128                  54247             19699 ns/op           42938 B/op         48 allocs/op
BenchmarkJoinLongData-128                  34216             31447 ns/op           46316 B/op         75 allocs/op
BenchmarkJoinShortData-128               3351846               335.7 ns/op           312 B/op          0 allocs/op
BenchmarkJoinShortData-128               4782178               220.1 ns/op           309 B/op          0 allocs/op
BenchmarkJoinShortData-128               2409987               425.7 ns/op           323 B/op          1 allocs/op
BenchmarkStdWrapLongData-128                  38          27880411 ns/op        137966772 B/op     70084 allocs/op
BenchmarkStdWrapLongData-128                  38          26452982 ns/op        137953649 B/op     70085 allocs/op
BenchmarkStdWrapLongData-128                  50          20250260 ns/op        137281848 B/op     53914 allocs/op
BenchmarkStdWrapShortData-128              43831             23024 ns/op           10736 B/op         70 allocs/op
BenchmarkStdWrapShortData-128              71410             14347 ns/op           13066 B/op         47 allocs/op
BenchmarkStdWrapShortData-128              55992             18342 ns/op           10656 B/op         57 allocs/op
BenchmarkStdJoinLongData-128               93657             11985 ns/op           29765 B/op       1051 allocs/op
BenchmarkStdJoinLongData-128               13926             73825 ns/op           36024 B/op       1207 allocs/op
BenchmarkStdJoinLongData-128               11421             87668 ns/op           37639 B/op       1248 allocs/op
BenchmarkStdJoinShortData-128            1830832               549.7 ns/op           279 B/op          9 allocs/op
BenchmarkStdJoinShortData-128            1901421               552.7 ns/op           277 B/op          9 allocs/op
BenchmarkStdJoinShortData-128            1101970               917.7 ns/op           316 B/op         10 allocs/op
PASS
ok      github.com/vdaas/vald/internal/errors   873.150s

Related Issue:

Versions:

Go Version: 1.20.3
Docker Version: 20.10.8
Kubernetes Version: 1.22.0
NGT Version: 2.0.9

Checklist:

I have read the CONTRIBUTING document and completed our CLA agreement.
I have checked open Pull Requests for the similar feature or fixes?

Special notes for your reviewer:

Signed-off-by: kpango <kpango@vdaas.org>

vdaas-ci · 2023-04-19T06:08:45Z

[CHATOPS:HELP] ChatOps commands.

🙆‍♀️ /approve - approve
💌 /changelog - replace the PR body by changelog details
🍱 /format - format codes and add licenses
✅ /gen-test - generate test codes
🏷️ /label - add labels
⏪ /rebase - rebase main
🔚 2️⃣ 🔚 /label actions/e2e-deploy - run E2E deploy & integration test

internal/errors/errors_benchmark_test.go

hlts2

LGTM

internal/errors/errors_benchmark_test.go

Signed-off-by: kpango <kpango@vdaas.org>

cloudflare-workers-and-pages · 2023-04-19T06:45:17Z

Deploying with Cloudflare Pages

Latest commit:	`1331c00`
Status:	✅ Deploy successful!
Preview URL:	https://547a7f38.vald.pages.dev
Branch Preview URL:	https://refactor-errors-improve-erro.vald.pages.dev

View logs

codecov · 2023-04-19T06:49:29Z

Codecov Report

Patch coverage: 16.90% and project coverage change: -0.05 ⚠️

Comparison is base (540c93a) 29.51% compared to head (1331c00) 29.46%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2010      +/-   ##
==========================================
- Coverage   29.51%   29.46%   -0.05%     
==========================================
  Files         366      366              
  Lines       34532    34559      +27     
==========================================
- Hits        10192    10184       -8     
- Misses      23917    23948      +31     
- Partials      423      427       +4

Impacted Files	Coverage Δ
internal/errors/errors.go	`50.24% <0.00%> (-3.13%)`	⬇️
internal/slices/slices.go	`33.33% <14.28%> (-66.67%)`	⬇️
internal/net/grpc/stream.go	`27.21% <40.00%> (+1.86%)`	⬆️

... and 7 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

hlts2

LGTM

github-actions · 2023-04-19T07:04:20Z

[FOSSA] The scan result will be available at https://app.fossa.com/projects/custom%2B21465%2Fvald/refs/branch/refactor%2Ferrors%2Fimprove-errors-join-performance/b98169dafe39a672c2edafcb27e380b8149df322

vdaas-ci · 2023-04-19T07:17:00Z

Profile Report

type	vald-agent-ngt	vald-lb-gateway	vald-discoverer	vald-manager-index
cpu
heap

other images

internal/slices/slices.go

sonatype-lift · 2023-04-19T09:34:42Z

internal/slices/slices_test.go

+
+func TestSortStableFunc(t *testing.T) {
+	type args struct {
+		x    []E


typecheck: undefined: E

❗❗ 2 similar findings have been found in this PR

🔎 Expand here to view all instances of this finding

File Path Line Number

internal/slices/slices_test.go 197

internal/slices/slices_test.go 279

Visit the Lift Web Console to find more details in your report.

ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage

@sonatype-lift ignore Leave out the above finding from this PR

@sonatype-lift ignoreall Leave out all the existing findings from this PR

@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
_{Click here to add LiftBot to another repo.}

Help us improve LIFT! (Sonatype LiftBot external survey)

Was this a good recommendation for you? _{Answering this survey will not impact your Lift settings.}

[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

Signed-off-by: kpango <kpango@vdaas.org>

sonatype-lift · 2023-04-19T10:03:51Z

internal/slices/slices_test.go

+		return nil
+	}
+	tests := []test{
+		// TODO test cases


godox: internal/slices/slices_test.go:303: Line contains TODO/BUG/FIXME: "TODO test cases"

ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage

@sonatype-lift ignore Leave out the above finding from this PR

@sonatype-lift ignoreall Leave out all the existing findings from this PR

@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
_{Click here to add LiftBot to another repo.}

Help us improve LIFT! (Sonatype LiftBot external survey)

Was this a good recommendation for you? _{Answering this survey will not impact your Lift settings.}

[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

sonatype-lift · 2023-04-19T10:03:53Z

internal/slices/slices_test.go

+		   },
+		*/
+
+		// TODO test cases


godox: internal/slices/slices_test.go:322: Line contains TODO/BUG/FIXME: "TODO test cases"

ℹ️ Expand to see all @sonatype-lift commands

You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.

Command Usage

@sonatype-lift ignore Leave out the above finding from this PR

@sonatype-lift ignoreall Leave out all the existing findings from this PR

@sonatype-lift exclude <file|issue|path|tool> Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file

Note: When talking to LiftBot, you need to refresh the page to see its response.
_{Click here to add LiftBot to another repo.}

Help us improve LIFT! (Sonatype LiftBot external survey)

Was this a good recommendation for you? _{Answering this survey will not impact your Lift settings.}

[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]

github-actions · 2023-04-19T10:19:53Z

internal/errors/errors.go

+		sb.Reset()
+		sbPool.Put(sb)
+	}()
+	sb.Grow(len(e.errs) * 16)


🚫 [golangci] _{reported by reviewdog 🐶}
mnd: Magic number: 16, in detected (gomnd)

github-actions · 2023-04-19T10:19:53Z

internal/net/grpc/stream.go

+	var (
+		mu   sync.Mutex
+		emu  sync.Mutex
+		errs = make([]error, 0, concurrency*2) // concurrency * recv+send


🚫 [golangci] _{reported by reviewdog 🐶}
mnd: Magic number: 2, in detected (gomnd)

internal/slices/slices.go

github-actions · 2023-04-20T04:20:35Z

[FOSSA] The scan result will be available at https://app.fossa.com/projects/custom%2B21465%2Fvald/refs/branch/refactor%2Ferrors%2Fimprove-errors-join-performance/79d39deb4bcb25d0e56dffc452919858c7c85d2b

vdaas-ci · 2023-04-20T04:28:45Z

Profile Report

type	vald-agent-ngt	vald-lb-gateway	vald-discoverer	vald-manager-index
cpu
heap

other images

[Refactor] Improve errors.Join performance

2c16e7d

Signed-off-by: kpango <kpango@vdaas.org>

github-actions bot added area/internal team/set SET team labels Apr 19, 2023

github-actions bot added size/L type/refactoring priority/low labels Apr 19, 2023

kpango requested review from a team, hlts2 and ykadowak and removed request for a team April 19, 2023 06:18

sonatype-lift bot reviewed Apr 19, 2023

View reviewed changes

internal/errors/errors_benchmark_test.go Show resolved Hide resolved

sonatype-lift bot reviewed Apr 19, 2023

View reviewed changes

internal/errors/errors_benchmark_test.go Show resolved Hide resolved

hlts2 previously approved these changes Apr 19, 2023

View reviewed changes

ykadowak previously approved these changes Apr 19, 2023

View reviewed changes

github-actions bot reviewed Apr 19, 2023

View reviewed changes

internal/errors/errors_benchmark_test.go Show resolved Hide resolved

internal/errors/errors_benchmark_test.go Show resolved Hide resolved

update golangci-lint configuration

4044ca2

Signed-off-by: kpango <kpango@vdaas.org>

kpango dismissed stale reviews from ykadowak and hlts2 via 4044ca2 April 19, 2023 06:45

ykadowak previously approved these changes Apr 19, 2023

View reviewed changes

hlts2 previously approved these changes Apr 19, 2023

View reviewed changes

kpango added actions/e2e-deploy actions/fossa actions/e2e-chaos actions/e2e-profiling actions/e2e-max-dim labels Apr 19, 2023

sonatype-lift bot reviewed Apr 19, 2023

View reviewed changes

internal/slices/slices.go Show resolved Hide resolved

sonatype-lift bot reviewed Apr 19, 2023

View reviewed changes

internal/slices/slices.go Show resolved Hide resolved

sonatype-lift bot reviewed Apr 19, 2023

View reviewed changes

internal/slices/slices.go Show resolved Hide resolved

kpango force-pushed the refactor/errors/improve-errors-join-performance branch from b2cdada to ccd4f81 Compare April 19, 2023 09:21

github-actions bot added area/makefile team/sre SRE team labels Apr 19, 2023

sonatype-lift bot reviewed Apr 19, 2023

View reviewed changes

kpango force-pushed the refactor/errors/improve-errors-join-performance branch from ccd4f81 to 66b3d38 Compare April 19, 2023 09:38

kpango removed actions/e2e-deploy actions/fossa actions/e2e-chaos actions/e2e-profiling actions/e2e-max-dim labels Apr 19, 2023

fix stream processing

1331c00

Signed-off-by: kpango <kpango@vdaas.org>

kpango force-pushed the refactor/errors/improve-errors-join-performance branch from 66b3d38 to 1331c00 Compare April 19, 2023 09:50

sonatype-lift bot reviewed Apr 19, 2023

View reviewed changes

github-actions bot reviewed Apr 19, 2023

View reviewed changes

hlts2 approved these changes Apr 19, 2023

View reviewed changes

vankichi approved these changes Apr 20, 2023

View reviewed changes

kpango added actions/e2e-deploy actions/fossa actions/e2e-chaos actions/e2e-profiling actions/e2e-max-dim labels Apr 20, 2023

kpango merged commit 538fbee into main Apr 20, 2023

kpango deleted the refactor/errors/improve-errors-join-performance branch April 20, 2023 04:58

ykadowak mentioned this pull request Apr 24, 2023

[patch] release v1.7.5 #2013

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Refactor] Improve errors.Join performance #2010

[Refactor] Improve errors.Join performance #2010

kpango commented Apr 19, 2023 •

edited

Loading

vdaas-ci commented Apr 19, 2023

hlts2 left a comment

cloudflare-workers-and-pages bot commented Apr 19, 2023 •

edited

Loading

codecov bot commented Apr 19, 2023 •

edited

Loading

hlts2 left a comment

github-actions bot commented Apr 19, 2023

vdaas-ci commented Apr 19, 2023

sonatype-lift bot Apr 19, 2023

sonatype-lift bot Apr 19, 2023

sonatype-lift bot Apr 19, 2023

github-actions bot Apr 19, 2023

github-actions bot Apr 19, 2023

github-actions bot commented Apr 20, 2023

vdaas-ci commented Apr 20, 2023

Command	Usage
`@sonatype-lift ignore`	Leave out the above finding from this PR
`@sonatype-lift ignoreall`	Leave out all the existing findings from this PR
`@sonatype-lift exclude <file\|issue\|path\|tool>`	Exclude specified `file\|issue\|path\|tool` from Lift findings by updating your config.toml file

[Refactor] Improve errors.Join performance #2010

[Refactor] Improve errors.Join performance #2010

Conversation

kpango commented Apr 19, 2023 • edited Loading

Description:

Related Issue:

Versions:

Checklist:

Special notes for your reviewer:

vdaas-ci commented Apr 19, 2023

hlts2 left a comment

Choose a reason for hiding this comment

cloudflare-workers-and-pages bot commented Apr 19, 2023 • edited Loading

Deploying with Cloudflare Pages

codecov bot commented Apr 19, 2023 • edited Loading

Codecov Report

hlts2 left a comment

Choose a reason for hiding this comment

github-actions bot commented Apr 19, 2023

vdaas-ci commented Apr 19, 2023

Profile Report

sonatype-lift bot Apr 19, 2023

Choose a reason for hiding this comment

sonatype-lift bot Apr 19, 2023

Choose a reason for hiding this comment

sonatype-lift bot Apr 19, 2023

Choose a reason for hiding this comment

github-actions bot Apr 19, 2023

Choose a reason for hiding this comment

github-actions bot Apr 19, 2023

Choose a reason for hiding this comment

github-actions bot commented Apr 20, 2023

vdaas-ci commented Apr 20, 2023

Profile Report

kpango commented Apr 19, 2023 •

edited

Loading

cloudflare-workers-and-pages bot commented Apr 19, 2023 •

edited

Loading

codecov bot commented Apr 19, 2023 •

edited

Loading