- 
                Notifications
    You must be signed in to change notification settings 
- Fork 312
Description
PR #993 creates a performance regression because go list is expensive and will be called on each package (even when range-val-address is not used).
"Funny thing", the regression impacts all the rules except range-val-address because this rule is skipped with go1.22.
It will be a performance regression for revive and for golangci-lint.
For golangci-lint, a way to define the Go version externally to bypass the go list is a solution.
I don't know enough about the revive design to propose a solution for revive.
We encountered the same problem with gosec golangci/golangci-lint#4735
log the call to detectGoVersion on revive's code
$ cd revive
$ ./revive ./...
detectGoVersion
detectGoVersion
detectGoVersion
detectGoVersion
detectGoVersion
detectGoVersion
detectGoVersion
detectGoVersion
detectGoVersion
detectGoVersion
detectGoVersion
rule/datarace.go:83:3: var getIds should be getIDs
lint/file.go:191:22: parameter 'filename' seems to be unused, consider removing or renaming it as _benchmark on kubernetes
# sample.toml
ignoreGeneratedHeader = false
severity = "warning"
confidence = 0.8
errorCode = 0
warningCode = 0
#[rule.range-val-address]
[rule.context-as-argument]$ cd kubernetes
$ rm go.work go.work.sum
$ hyperfine './revive-bbe5eb7  -config sample.toml ./...' './revive-4242f24  -config sample.toml ./...'
Benchmark 1: ./revive-bbe5eb7  -config sample.toml ./...
  Time (mean ± σ):      1.989 s ±  0.125 s    [User: 11.202 s, System: 1.519 s]
  Range (min … max):    1.796 s …  2.220 s    10 runs
 
Benchmark 2: ./revive-4242f24  -config sample.toml ./...
  Time (mean ± σ):      3.880 s ±  0.171 s    [User: 22.235 s, System: 14.092 s]
  Range (min … max):    3.633 s …  4.193 s    10 runs
 
Summary
  ./revive-bbe5eb7  -config sample.toml ./... ran
    1.95 ± 0.15 times faster than ./revive-4242f24  -config sample.toml ./...$ cd revive
$ git lg
* 4242f24 - Add support for the new implementation of for loop variables in go 1.22. (#993)
* bbe5eb7 - fix(deps): update module github.com/burntsushi/toml to v1.4.0 (#992)
...With the commit, the run of context-as-argument (just an example) is about 2x slower than the previous commit (and about 10x slower on the system).
Also, the implementation has a bug with Go workspaces:
can't parse the output of go list: invalid character '{' after top-level value
Inside a Go workspace, go list always returns all the modules, not just the current module.
As a reference, a working implementation: https://github.com/golangci/modinfo/blob/main/module.go
$ cd kubernetes
$ go list -m -json                                
{
        "Path": "k8s.io/kubernetes",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/api",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/api",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/api/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/apiextensions-apiserver",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/apiextensions-apiserver",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/apiextensions-apiserver/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/apimachinery",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/apimachinery",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/apimachinery/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/apiserver",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/apiserver",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/apiserver/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/cli-runtime",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/cli-runtime",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/cli-runtime/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/client-go",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/client-go",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/client-go/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/cloud-provider",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/cloud-provider",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/cloud-provider/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/cluster-bootstrap",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/cluster-bootstrap",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/cluster-bootstrap/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/code-generator",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/code-generator",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/code-generator/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/component-base",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/component-base",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/component-base/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/component-helpers",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/component-helpers",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/component-helpers/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/controller-manager",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/controller-manager",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/controller-manager/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/cri-api",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/cri-api",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/cri-api/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/cri-client",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/cri-client",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/cri-client/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/csi-translation-lib",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/csi-translation-lib",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/csi-translation-lib/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/dynamic-resource-allocation",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/dynamic-resource-allocation",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/dynamic-resource-allocation/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/endpointslice",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/endpointslice",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/endpointslice/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/kms",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kms",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kms/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/kube-aggregator",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kube-aggregator",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kube-aggregator/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/kube-controller-manager",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kube-controller-manager",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kube-controller-manager/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/kube-proxy",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kube-proxy",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kube-proxy/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/kube-scheduler",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kube-scheduler",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kube-scheduler/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/kubectl",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kubectl",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kubectl/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/kubelet",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kubelet",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/kubelet/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/metrics",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/metrics",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/metrics/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/mount-utils",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/mount-utils",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/mount-utils/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/pod-security-admission",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/pod-security-admission",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/pod-security-admission/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/sample-apiserver",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/sample-apiserver",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/sample-apiserver/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/sample-cli-plugin",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/sample-cli-plugin",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/sample-cli-plugin/go.mod",
        "GoVersion": "1.22.0"
}
{
        "Path": "k8s.io/sample-controller",
        "Main": true,
        "Dir": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/sample-controller",
        "GoMod": "/home/ldez/sources/experimental/kubernetes/staging/src/k8s.io/sample-controller/go.mod",
        "GoVersion": "1.22.0"
}
FYI, the issue comment (golang/go#44753 (comment)), used as a reference inside the PR, is outdated since Go workspace (go1.18).