refactor: replace mutex with sync.Once for rule configuration

[alexandear]

This PR refactors the rules' implementation by using sync.Once instead of sync.Mutex.

sync.Mutex was previously used only to ensure that rule parameters are configured only once.

The main reason for refactoring is to simplify the code and reduce it by 180 lines.

Great blog post about sync.Once: https://victoriametrics.com/blog/go-sync-once/

[ccoVeille]

Thanks for refactoring!

[ccoVeille]

Did you see any performance improvement?

[denisvmedia]

Yeah, I'd second the question. I guess we should measure the difference.

[alexandear]

Did you see any performance improvement?

I didn't measure the performance. The main reason for refactoring is to simplify rules' implementation.

I'm not sure that we gain any improvement with this refactoring.

[alexandear]

Yeah, I'd second the question. I guess we should measure the difference.

OK, I'll measure the difference soon.

[denisvmedia]

I wrote this test:

package main

import (
        "sync"
        "testing"
)

var (
        once sync.Once
        mu   sync.Mutex
        done bool
)

func initOnce() {
        once.Do(func() {
                // Simulate some work
        })
}

func initMutex() {
        mu.Lock()
        defer mu.Unlock()
        if !done {
                // Simulate some work
                done = true
        }
}

// Benchmark for sync.Once
func BenchmarkSyncOnce(b *testing.B) {
        // Resetting `once` to allow it to re-run in each benchmark iteration
        for i := 0; i < b.N; i++ {
                once = sync.Once{}
                initOnce()
        }
}

// Benchmark for sync.Mutex
func BenchmarkSyncMutex(b *testing.B) {
        for i := 0; i < b.N; i++ {
                done = false
                initMutex()
        }
}

For me it runs faster for Mutex (maybe I'm missing something though):

$ go test -bench=.
goos: darwin
goarch: arm64
pkg: testpkg
cpu: Apple M3 Pro
BenchmarkSyncOnce-12     	213841928	         5.546 ns/op
BenchmarkSyncMutex-12    	305101034	         3.808 ns/op
PASS
ok  	testpkg	3.489s

[denisvmedia]

However, if I modify the test, the results are the opposite:

package main

import (
        "sync"
        "testing"
)

var (
        once sync.Once
        mu   sync.Mutex
        done bool
)

func initOnce() {
        once.Do(func() {
                // Simulate some work
        })
}

func initMutex() {
        mu.Lock()
        defer mu.Unlock()
        if !done {
                // Simulate some work
                done = true
        }
}

// Benchmark for sync.Once
func BenchmarkSyncOnce(b *testing.B) {
        // Resetting `once` to allow it to re-run in each benchmark iteration
        for i := 0; i < b.N; i++ {
                once = sync.Once{}
                initOnce()
                initOnce()
        }
}

// Benchmark for sync.Mutex
func BenchmarkSyncMutex(b *testing.B) {
        for i := 0; i < b.N; i++ {
                done = false
                initMutex()
                initMutex()
        }
}

go test -bench=.
goos: darwin
goarch: arm64
pkg: testpkg
cpu: Apple M3 Pro
BenchmarkSyncOnce-12     	197480839	         5.914 ns/op
BenchmarkSyncMutex-12    	154100996	         7.867 ns/op
PASS
ok  	testpkg	3.856s

[denisvmedia]

So, it means that for the first call, Mutex wins, but later on Once wins.

[alexandear]

I run revive over the beego repo using this command:

❯ for i in $(seq 1 5); do time (./revive -config revive.toml ./... > /dev/null); done

Configuration revive.toml

ignoreGeneratedHeader = false
severity = "warning"
confidence = 0.8
errorCode = 1
warningCode = 1

[rule.add-constant]
[rule.argument-limit]
  arguments = [40]
[rule.atomic]
[rule.banned-characters]
  arguments = ["Ω","Σ","σ"]
[rule.bare-return]
[rule.blank-imports]
[rule.bool-literal-in-expr]
[rule.call-to-gc]
[rule.cognitive-complexity]
  arguments = [700]
[rule.comment-spacings]
  arguments = ["mypragma:", "+optional"]
[rule.comments-density]
  arguments = [100]
[rule.confusing-naming]
[rule.confusing-results]
[rule.constant-logical-expr]
[rule.context-as-argument]
[rule.context-keys-type]
[rule.cyclomatic]
  arguments = [100]
[rule.datarace]
[rule.deep-exit]
[rule.defer]
  arguments = [["call-chain","loop"]]
[rule.dot-imports]
  arguments = [{ allowedPackages = ["github.com/onsi/ginkgo/v2","github.com/onsi/gomega"] }]
[rule.duplicated-imports]
[rule.early-return]
[rule.empty-block]
[rule.empty-lines]
[rule.enforce-map-style]
  arguments = ["make"]
[rule.enforce-repeated-arg-type-style]
  arguments = ["short"]
[rule.enforce-slice-style]
  arguments = ["make"]
[rule.error-naming]
[rule.error-return]
[rule.error-strings]
[rule.errorf]
[rule.exported]
[rule.file-header]
  arguments = ["This is the text that must appear at the top of source files."]
[rule.file-length-limit]
  arguments = [{max=100}]
[rule.filename-format]
[rule.flag-parameter]
[rule.function-length]
  arguments = [100, 100]
[rule.function-result-limit]
  arguments = [10]
[rule.get-return]
[rule.identical-branches]
[rule.if-return]
[rule.import-alias-naming]
  arguments = ["^[a-z][a-z0-9]{0,}$"]
[rule.import-shadowing]
[rule.imports-blocklist]
  arguments = ["crypto/md5", "crypto/sha1", "crypto/**/pkix"]
[rule.increment-decrement]
[rule.indent-error-flow]
[rule.line-length-limit]
    arguments = [130]
[rule.max-control-nesting]
[rule.max-public-structs]
  arguments = [100]
[rule.modifies-parameter]
[rule.modifies-value-receiver]
[rule.nested-structs]
[rule.optimize-operands-order]
[rule.package-comments]
[rule.range-val-address]
[rule.range-val-in-closure]
[rule.range]
[rule.receiver-naming]
    arguments = [{maxLength=10}]
[rule.redefines-builtin-id]
[rule.redundant-import-alias]
[rule.string-format]
  arguments = [
    ["core.WriteError[1].Message", "/^([^A-Z]|$)/", "must not start with a capital letter"],
    ["fmt.Errorf[0]", "/(^|[^\\.!?])$/", "must not end in punctuation"],
    ["panic", "/^[^\\n]*$/", "must not contain line breaks"],
    ["fmt.Errorf[0],core.WriteError[1].Message", "!/^.*%w.*$/", "must not contain '%w'"],
  ]
[rule.string-of-int]
[rule.struct-tag]
  arguments = ["json,inline", "bson,outline,gnu"]
[rule.superfluous-else]
[rule.time-equal]
[rule.time-naming]
[rule.unchecked-type-assertion]
[rule.unconditional-recursion]
[rule.unexported-naming]
[rule.unexported-return]
[rule.unhandled-error]
[rule.unnecessary-stmt]
[rule.unreachable-code]
[rule.unused-parameter]
[rule.unused-receiver]
    arguments = [{ allowRegex = "^_" }]
[rule.use-any]
[rule.useless-break]
[rule.var-declaration]
[rule.var-naming]
  arguments = [["ID"], ["VM"], [{upperCaseConst=true}]]
[rule.waitgroup-by-value]

With sync.Mutex:

( ./revive-mutex -config revive.toml ./... > /dev/nul)  6.27s user 8.18s system 414% cpu 3.487 total
( ./revive-mutex -config revive.toml ./... > /dev/nul)  6.15s user 7.80s system 478% cpu 2.914 total
( ./revive-mutex -config revive.toml ./... > /dev/nul)  6.33s user 7.79s system 501% cpu 2.815 total
( ./revive-mutex -config revive.toml ./... > /dev/nul)  6.18s user 8.05s system 503% cpu 2.824 total
( ./revive-mutex -config revive.toml ./... > /dev/nul)  6.28s user 7.89s system 515% cpu 2.750 total

With sync.Once:

( ./revive-once -config revive.toml ./... > /dev/nul)  5.95s user 7.81s system 341% cpu 4.029 total
( ./revive-once -config revive.toml ./... > /dev/nul)  5.84s user 7.37s system 416% cpu 3.169 total
( ./revive-once -config revive.toml ./... > /dev/nul)  6.05s user 7.54s system 510% cpu 2.661 total
( ./revive-once -config revive.toml ./... > /dev/nul)  6.06s user 7.79s system 546% cpu 2.535 total
( ./revive-once -config revive.toml ./... > /dev/nul)  5.93s user 7.77s system 563% cpu 2.433 total

The version with the sync.Once is slightly faster. But this speed up is negligible.

[chavacava]

LGTM

[alexandear]

@chavacava I resolved conflicts, the PR can be merged.