Skip to content

Commit

Permalink
Update golangci-lint and refactor code after update to 1.21 standards (
Browse files Browse the repository at this point in the history
…#3313)

* Update golangci-lint and target go version 1.21

* Refactor deprecated code paths

* remove random seeding
* migrate from reflect.SliceHeader over to unsafe.
  • Loading branch information
simonswine committed May 24, 2024
1 parent 616e81a commit 53f7de2
Show file tree
Hide file tree
Showing 11 changed files with 38 additions and 86 deletions.
41 changes: 22 additions & 19 deletions .golangci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

# options for analysis running
run:
go: "1.19"
go: "1.21"
# default concurrency is a available CPU number
concurrency: 16

Expand All @@ -21,26 +21,11 @@ run:

modules-download-mode: readonly

# which dirs to skip: they won't be analyzed;
# can use regexp here: generated.*, regexp is applied on full path;
# default value is empty list, but next dirs are always skipped independently
# from this option's value:
# vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
skip-dirs:
- win_eventlog$
- pkg/og
# which files to skip: they will be analyzed, but issues from them
# won't be reported. Default value is empty list, but there is
# no need to include all autogenerated files, we confidently recognize
# autogenerated files. If it's not please let us know.
skip-files:
- .*.pb.go
- .*.y.go
- .*.rl.go
# output configuration options
output:
# colored-line-number|line-number|json|tab|checkstyle, default is "colored-line-number"
format: colored-line-number
formats:
- format: colored-line-number

# print lines of code with issue, default is true
print-issued-lines: true
Expand All @@ -50,7 +35,7 @@ output:

linters-settings:
goimports:
local-prefixes: github.com/grafana/pyroscope/pkg,github.com/grafana/pyroscope/api,github.com/grafana/pyroscope/tools
local-prefixes: github.com/grafana/pyroscope/,github.com/grafana/pyroscope/api,github.com/grafana/pyroscope/tools,github.com/grafana/pyroscope/ebpf

depguard:
rules:
Expand Down Expand Up @@ -87,3 +72,21 @@ issues:
- Error return value of .*log\.Logger\)\.Log\x60 is not checked
- Error return value of .*.Log.* is not checked
- Error return value of `` is not checked

# which dirs to skip: they won't be analyzed;
# can use regexp here: generated.*, regexp is applied on full path;
# default value is empty list, but next dirs are always skipped independently
# from this option's value:
# vendor$, third_party$, testdata$, examples$, Godeps$, builtin$
exclude-dirs:
- win_eventlog$
- pkg/og
# which files to skip: they will be analyzed, but issues from them
# won't be reported. Default value is empty list, but there is
# no need to include all autogenerated files, we confidently recognize
# autogenerated files. If it's not please let us know.
exclude-files:
- .*.pb.go
- .*.y.go
- .*.rl.go

2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -260,7 +260,7 @@ $(BIN)/buf: Makefile

$(BIN)/golangci-lint: Makefile
@mkdir -p $(@D)
GOBIN=$(abspath $(@D)) $(GO) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.54.0
GOBIN=$(abspath $(@D)) $(GO) install github.com/golangci/golangci-lint/cmd/golangci-lint@v1.58.2

$(BIN)/protoc-gen-go: Makefile go.mod
@mkdir -p $(@D)
Expand Down
4 changes: 0 additions & 4 deletions pkg/frontend/frontend_scheduler_worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,6 @@ import (
"github.com/grafana/pyroscope/pkg/util/servicediscovery"
)

func init() {
rand.Seed(time.Now().UnixNano())
}

const (
schedulerAddressLabel = "scheduler_address"
// schedulerWorkerCancelChanCapacity should be at least as big as the number of sub-queries issued by a single query
Expand Down
9 changes: 1 addition & 8 deletions pkg/model/stacktraces.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import (
"bytes"
"container/heap"
"io"
"reflect"
"sync"
"unsafe"

Expand Down Expand Up @@ -303,11 +302,5 @@ func (t *StacktraceTree) Bytes(dst io.Writer, maxNodes int64, funcs []string) {
}

func unsafeStringBytes(s string) []byte {
p := unsafe.Pointer((*reflect.StringHeader)(unsafe.Pointer(&s)).Data)
var b []byte
hdr := (*reflect.SliceHeader)(unsafe.Pointer(&b))
hdr.Data = uintptr(p)
hdr.Cap = len(s)
hdr.Len = len(s)
return b
return unsafe.Slice(unsafe.StringData(s), len(s))
}
15 changes: 4 additions & 11 deletions pkg/phlaredb/symdb/dedup_slice.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ package symdb
import (
"fmt"
"hash/maphash"
"reflect"
"sort"
"sync"
"unsafe"
Expand Down Expand Up @@ -424,23 +423,17 @@ func hashLines(s []schemav1.InMemoryLine) uint64 {
if len(s) == 0 {
return 0
}
var b []byte
hdr := (*reflect.SliceHeader)(unsafe.Pointer(&b))
hdr.Len = len(s) * int(lineSize)
hdr.Cap = hdr.Len
hdr.Data = uintptr(unsafe.Pointer(&s[0]))
p := (*byte)(unsafe.Pointer(&s[0]))
b := unsafe.Slice(p, len(s)*int(lineSize))
return maphash.Bytes(mapHashSeed, b)
}

func hashLocations(s []uint64) uint64 {
if len(s) == 0 {
return 0
}
var b []byte
hdr := (*reflect.SliceHeader)(unsafe.Pointer(&b))
hdr.Len = len(s) * 8
hdr.Cap = hdr.Len
hdr.Data = uintptr(unsafe.Pointer(&s[0]))
p := (*byte)(unsafe.Pointer(&s[0]))
b := unsafe.Slice(p, len(s)*8)
return maphash.Bytes(mapHashSeed, b)
}

Expand Down
11 changes: 4 additions & 7 deletions pkg/phlaredb/symdb/resolver_pprof_truncate.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package symdb

import (
"reflect"
"unsafe"

googlev1 "github.com/grafana/pyroscope/api/gen/proto/go/google/v1"
Expand Down Expand Up @@ -228,13 +227,11 @@ func truncateLocations(locations []uint64, functions []int32, offset int, symbol
}

func uint64sliceString(u []uint64) string {
var s string
if len(u) != 0 {
hdr := (*reflect.SliceHeader)(unsafe.Pointer(&s))
hdr.Data = uintptr(unsafe.Pointer(&u[0]))
hdr.Len = len(u) * 8
if len(u) == 0 {
return ""
}
return s
p := (*byte)(unsafe.Pointer(&u[0]))
return unsafe.String(p, len(u)*8)
}

func (r *pprofProtoTruncatedSymbols) createStubSample() {
Expand Down
8 changes: 2 additions & 6 deletions pkg/pprof/fix_go_heap_truncated.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ package pprof

import (
"bytes"
"reflect"
"slices"
"sort"
"unsafe"
Expand Down Expand Up @@ -224,11 +223,8 @@ func topToken(s []uint64) []byte {
}

func locBytes(s []uint64) []byte {
size := len(s) * 8
h := (*reflect.SliceHeader)(unsafe.Pointer(&s))
h.Len = size
h.Cap = size
return *(*[]byte)(unsafe.Pointer(h))
p := (*byte)(unsafe.Pointer(&s[0]))
return unsafe.Slice(p, len(s)*8)
}

func unsafeString(b []byte) string {
Expand Down
9 changes: 2 additions & 7 deletions pkg/pprof/pprof.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (
"encoding/hex"
"io"
"os"
"reflect"
"sort"
"strings"
"sync"
Expand Down Expand Up @@ -578,12 +577,8 @@ func uint64Bytes(s []uint64) []byte {
if len(s) == 0 {
return nil
}
var bs []byte
hdr := (*reflect.SliceHeader)(unsafe.Pointer(&bs))
hdr.Len = len(s) * 8
hdr.Cap = hdr.Len
hdr.Data = uintptr(unsafe.Pointer(&s[0]))
return bs
p := (*byte)(unsafe.Pointer(&s[0]))
return unsafe.Slice(p, len(s)*8)
}

type SamplesByLabels []*profilev1.Sample
Expand Down
4 changes: 0 additions & 4 deletions pkg/querier/worker/scheduler_processor.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,6 @@ import (
"github.com/grafana/pyroscope/pkg/util/httpgrpcutil"
)

func init() {
rand.Seed(time.Now().UnixNano())
}

var processorBackoffConfig = backoff.Config{
MinBackoff: 250 * time.Millisecond,
MaxBackoff: 2 * time.Second,
Expand Down
18 changes: 0 additions & 18 deletions pkg/util/unsafe.go

This file was deleted.

3 changes: 2 additions & 1 deletion public/execute_template_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@ import (
"strings"
"testing"

"github.com/grafana/pyroscope/public"
"github.com/stretchr/testify/assert"

"github.com/grafana/pyroscope/public"
)

func TestInjectingBaseURL(t *testing.T) {
Expand Down

0 comments on commit 53f7de2

Please sign in to comment.