Skip to content
New issue

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

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

Already on GitHub? # to your account

unused-param,unused-receiver: refactor configure func #1157

Merged
merged 1 commit into from
Dec 4, 2024

Conversation

alexandear
Copy link
Contributor

The PR refactors the configure function in both unused-param and unused-receiver:

  • change to use blankIdentifierRegex that is compiled once;
  • reduce nested ifs;
  • add tests and benchmarks.

Benchmarks

Benchmarks show that version in this PR has slightly fewer allocations.

$ git switch master
$ go test -benchmem -run=^$ -bench "^BenchmarkUnused.*" github.com/mgechev/revive/test -count=10 > old.txt
$ git switch refactor/unused-configure
$ go test -benchmem -run=^$ -bench "^BenchmarkUnused.*" github.com/mgechev/revive/test -count=10 > new.txt
$ benchstat old.txt new.txt
goos: darwin
goarch: arm64
pkg: github.com/mgechev/revive/test
cpu: Apple M1 Pro
                 │   old.txt   │              new.txt               │
                 │   sec/op    │   sec/op     vs base               │
UnusedParam-8      671.0µ ± 1%   673.0µ ± 3%       ~ (p=0.436 n=10)
UnusedReceiver-8   211.9µ ± 8%   207.7µ ± 4%       ~ (p=0.063 n=10)
geomean            377.0µ        373.9µ       -0.83%

                 │   old.txt    │               new.txt               │
                 │     B/op     │     B/op      vs base               │
UnusedParam-8      325.5Ki ± 0%   323.7Ki ± 0%  -0.58% (p=0.000 n=10)
UnusedReceiver-8   80.22Ki ± 0%   78.09Ki ± 0%  -2.65% (p=0.000 n=10)
geomean            161.6Ki        159.0Ki       -1.62%

                 │   old.txt   │              new.txt               │
                 │  allocs/op  │  allocs/op   vs base               │
UnusedParam-8      8.322k ± 0%   8.290k ± 0%  -0.38% (p=0.000 n=10)
UnusedReceiver-8   1.896k ± 0%   1.864k ± 0%  -1.69% (p=0.000 n=10)
geomean            3.972k        3.931k       -1.04%

@alexandear alexandear force-pushed the refactor/unused-configure branch from 6cee6c4 to 04f5c62 Compare December 3, 2024 16:57
@chavacava chavacava merged commit 09fb350 into mgechev:master Dec 4, 2024
5 checks passed
@alexandear alexandear deleted the refactor/unused-configure branch December 4, 2024 08:45
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants