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

refactor(sumologicexporter): use golang.org/x/exp/slices for sorting fields #519

Merged

Conversation

pmalek-sumo
Copy link
Contributor

@pmalek-sumo pmalek-sumo commented Apr 4, 2022

This PR uses generic slice sorting algorithm and saves yet another allocation in fields.string().

Related article: https://eli.thegreenplace.net/2022/faster-sorting-with-go-generics/

benchcmp old.txt new.txt
benchcmp is deprecated in favor of benchstat: https://pkg.go.dev/golang.org/x/perf/cmd/benchstat
benchmark              old ns/op     new ns/op     delta
BenchmarkFields-16     646           618           -4.32%
BenchmarkFields-16     642           603           -6.03%
BenchmarkFields-16     636           614           -3.35%
BenchmarkFields-16     640           616           -3.72%
BenchmarkFields-16     648           612           -5.56%

benchmark              old allocs     new allocs     delta
BenchmarkFields-16     11             10             -9.09%
BenchmarkFields-16     11             10             -9.09%
BenchmarkFields-16     11             10             -9.09%
BenchmarkFields-16     11             10             -9.09%
BenchmarkFields-16     11             10             -9.09%

benchmark              old bytes     new bytes     delta
BenchmarkFields-16     368           344           -6.52%
BenchmarkFields-16     368           344           -6.52%
BenchmarkFields-16     368           344           -6.52%
BenchmarkFields-16     368           344           -6.52%
BenchmarkFields-16     368           344           -6.52%

EDIT: the linter fails because we don't have full generics support in golangci-lint just yet: golangci/golangci-lint#2649

@pmalek-sumo pmalek-sumo self-assigned this Apr 4, 2022
@github-actions github-actions bot added the go label Apr 4, 2022
@pmalek-sumo pmalek-sumo marked this pull request as ready for review April 4, 2022 12:38
@pmalek-sumo pmalek-sumo requested a review from a team as a code owner April 4, 2022 12:38
@swiatekm
Copy link

swiatekm commented Apr 5, 2022

I'm not sure if I'm reading this right, but from golangci/golangci-lint#2649 it sounds like the linter should work if we explicitly set go version to 1.18 in the config? The issue description claims that they automatically disable linters which can't handle generics for go 1.18. @pmalek-sumo ?

@pmalek-sumo pmalek-sumo force-pushed the sumologicexporter-use-generic-sort-for-performance-gains branch from 96c768b to c32b5ff Compare April 5, 2022 14:02
@pmalek-sumo
Copy link
Contributor Author

I'm not sure if I'm reading this right, but from golangci/golangci-lint#2649 it sounds like the linter should work if we explicitly set go version to 1.18 in the config? The issue description claims that they automatically disable linters which can't handle generics for go 1.18. @pmalek-sumo ?

It seems that

  • unused
  • gosimple
  • staticcheck

are the ones that cause problems. I've disables specifically those for now in config and linter should be happy now. @swiatekm-sumo

@pmalek-sumo pmalek-sumo enabled auto-merge (squash) April 5, 2022 14:07
@pmalek-sumo pmalek-sumo force-pushed the sumologicexporter-use-generic-sort-for-performance-gains branch from c32b5ff to 9686a44 Compare April 5, 2022 14:35
@pmalek-sumo pmalek-sumo merged commit 8137956 into main Apr 5, 2022
@pmalek-sumo pmalek-sumo deleted the sumologicexporter-use-generic-sort-for-performance-gains branch April 5, 2022 14:47
kasia-kujawa added a commit that referenced this pull request May 16, 2022
kasia-kujawa added a commit that referenced this pull request May 16, 2022
kasia-kujawa added a commit that referenced this pull request May 19, 2022
kasia-kujawa added a commit that referenced this pull request May 19, 2022
kasia-kujawa added a commit that referenced this pull request May 19, 2022
kasia-kujawa added a commit that referenced this pull request May 19, 2022
kasia-kujawa added a commit that referenced this pull request May 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants