Skip to content

Commit

Permalink
refactor: replace min/max helpers with built-in min/max (#3899)
Browse files Browse the repository at this point in the history
We can use the built-in `min` and `max` functions since Go 1.21.

Reference: https://go.dev/ref/spec#Min_and_max

Signed-off-by: Eng Zer Jun <[email protected]>
  • Loading branch information
Juneezee authored Feb 11, 2025
1 parent c85bda3 commit b5d11eb
Show file tree
Hide file tree
Showing 13 changed files with 18 additions and 90 deletions.
5 changes: 2 additions & 3 deletions pkg/experiment/ingester/segment.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import (
"github.com/grafana/pyroscope/pkg/model"
pprofsplit "github.com/grafana/pyroscope/pkg/model/pprof_split"
pprofmodel "github.com/grafana/pyroscope/pkg/pprof"
"github.com/grafana/pyroscope/pkg/util/math"
"github.com/grafana/pyroscope/pkg/validation"
)

Expand Down Expand Up @@ -268,8 +267,8 @@ func (s *segment) flushBlock(heads []flushedServiceHead) ([]byte, *metastorev1.B
meta.MinTime = svc.MinTime
meta.MaxTime = svc.MaxTime
} else {
meta.MinTime = math.Min(meta.MinTime, svc.MinTime)
meta.MaxTime = math.Max(meta.MaxTime, svc.MaxTime)
meta.MinTime = min(meta.MinTime, svc.MinTime)
meta.MaxTime = max(meta.MaxTime, svc.MaxTime)
}
s.sw.metrics.headSizeBytes.WithLabelValues(s.sshard, e.key.tenant).Observe(float64(svc.Size))
meta.Datasets = append(meta.Datasets, svc)
Expand Down
13 changes: 3 additions & 10 deletions pkg/frontend/dot/graph/dotgraph.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,10 +291,10 @@ func (b *builder) addEdge(edge *Edge, from, to int, hasNodelets bool) {
attr := fmt.Sprintf(`label=" %s%s"`, w, inline)
if b.config.Total != 0 {
// Note: edge.weight > b.config.Total is possible for profile diffs.
if weight := 1 + int(min64(abs64(edge.WeightValue()*100/b.config.Total), 100)); weight > 1 {
if weight := 1 + int(min(abs64(edge.WeightValue()*100/b.config.Total), 100)); weight > 1 {
attr = fmt.Sprintf(`%s weight=%d`, attr, weight)
}
if width := 1 + int(min64(abs64(edge.WeightValue()*5/b.config.Total), 5)); width > 1 {
if width := 1 + int(min(abs64(edge.WeightValue()*5/b.config.Total), 5)); width > 1 {
attr = fmt.Sprintf(`%s penwidth=%d`, attr, width)
}
attr = fmt.Sprintf(`%s color="%s"`, attr,
Expand Down Expand Up @@ -354,7 +354,7 @@ func dotColor(score float64, isBackground bool) string {
}

// Limit the score values to the range [-1.0, 1.0].
score = math.Max(-1.0, math.Min(1.0, score))
score = max(-1.0, min(1.0, score))

// Reduce saturation near score=0 (so it is colored grey, rather than yellow).
if math.Abs(score) < 0.2 {
Expand Down Expand Up @@ -470,13 +470,6 @@ func (b *builder) tagGroupLabel(g []*Tag) (label string, flat, cum int64) {
return measurement.Label(min.Value, min.Unit) + ".." + measurement.Label(max.Value, max.Unit), f, c
}

func min64(a, b int64) int64 {
if a < b {
return a
}
return b
}

// escapeAllForDot applies escapeForDot to all strings in the given slice.
func escapeAllForDot(in []string) []string {
var out = make([]string, len(in))
Expand Down
4 changes: 1 addition & 3 deletions pkg/frontend/split_by_interval.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@ package frontend

import (
"time"

"github.com/grafana/pyroscope/pkg/util/math"
)

// TimeIntervalIterator splits a time range into non-overlapping sub-ranges,
Expand Down Expand Up @@ -50,7 +48,7 @@ func NewTimeIntervalIterator(startTime, endTime time.Time, interval time.Duratio
for _, option := range options {
option(i)
}
i.interval = math.Max(i.interval, i.alignment)
i.interval = max(i.interval, i.alignment)
return i
}

Expand Down
25 changes: 2 additions & 23 deletions pkg/model/flamegraph_diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,7 @@ func combineTree(leftTree, rightTree *Tree) (*Tree, *Tree) {
// by filling with non existing nodes
// and sorting lexicographically
func combineNodes(leftNodes, rghtNodes []*node) ([]*node, []*node) {
size := nextPow2(maxInt(len(leftNodes), len(rghtNodes)))
size := nextPow2(max(len(leftNodes), len(rghtNodes)))
leftResult := make([]*node, 0, size)
rghtResult := make([]*node, 0, size)

Expand Down Expand Up @@ -244,27 +244,6 @@ func combineNodes(leftNodes, rghtNodes []*node) ([]*node, []*node) {
return leftResult, rghtResult
}

func maxUint64(a, b uint64) uint64 {
if a > b {
return a
}
return b
}

func maxInt(a, b int) int {
if a > b {
return a
}
return b
}

func max(a, b int64) int64 {
if a > b {
return a
}
return b
}

func nextPow2(a int) int {
a--
a |= a >> 1
Expand All @@ -289,7 +268,7 @@ func combineMinValues(leftTree, rightTree *Tree, maxNodes int) uint64 {
}
c := cappedarr.New(maxNodes)
combineIterateWithTotal(leftTree, rightTree, func(left uint64, right uint64) bool {
return c.Push(maxUint64(left, right))
return c.Push(max(left, right))
})
return c.MinValue()
}
Expand Down
16 changes: 1 addition & 15 deletions pkg/og/storage/tree/treediff.go
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,7 @@ func CombineToFlamebearerStruct(leftTree, rightTree *Tree, maxNodes int) *Flameb
func combineMinValues(leftTree, rightTree *Tree, maxNodes int) uint64 {
c := cappedarr.New(maxNodes)
combineIterateWithTotal(leftTree, rightTree, func(left uint64, right uint64) bool {
return c.Push(maxUint64(left, right))
return c.Push(max(left, right))
})
return c.MinValue()
}
Expand All @@ -211,20 +211,6 @@ func combineIterateWithTotal(leftTree, rightTree *Tree, cb func(uint64, uint64)
}
}

func maxUint64(a, b uint64) uint64 {
if a > b {
return a
}
return b
}

func max(a, b int) int {
if a > b {
return a
}
return b
}

func nextPow2(a int) int {
a--
a |= a >> 1
Expand Down
5 changes: 2 additions & 3 deletions pkg/phlaredb/symdb/format.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
"github.com/parquet-go/parquet-go/encoding/delta"

"github.com/grafana/pyroscope/pkg/slices"
"github.com/grafana/pyroscope/pkg/util/math"
)

// V1 and V2:
Expand Down Expand Up @@ -769,7 +768,7 @@ func newSymbolsEncoder[T any](e symbolsBlockEncoder[T]) *symbolsEncoder[T] {
func (e *symbolsEncoder[T]) encode(w io.Writer, items []T) (err error) {
l := len(items)
for i := 0; i < l; i += e.blockSize {
block := items[i:math.Min(i+e.blockSize, l)]
block := items[i:min(i+e.blockSize, l)]
if err = e.blockEncoder.encode(w, block); err != nil {
return err
}
Expand Down Expand Up @@ -800,7 +799,7 @@ func (d *symbolsDecoder[T]) decode(dst []T, r io.Reader) error {
blocks := int((d.h.Length + d.h.BlockSize - 1) / d.h.BlockSize)
for i := 0; i < blocks; i++ {
lo := i * int(d.h.BlockSize)
hi := math.Min(lo+int(d.h.BlockSize), int(d.h.Length))
hi := min(lo+int(d.h.BlockSize), int(d.h.Length))
block := dst[lo:hi]
if err := d.d.decode(r, block); err != nil {
return fmt.Errorf("malformed block (format %d): %w", d.h.Format, err)
Expand Down
3 changes: 1 addition & 2 deletions pkg/phlaredb/symdb/functions.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (

v1 "github.com/grafana/pyroscope/pkg/phlaredb/schemas/v1"
"github.com/grafana/pyroscope/pkg/slices"
"github.com/grafana/pyroscope/pkg/util/math"
)

var (
Expand Down Expand Up @@ -135,7 +134,7 @@ type functionsBlockDecoder struct {

func newFunctionsDecoder(h SymbolsBlockHeader) (*symbolsDecoder[v1.InMemoryFunction], error) {
if h.Format == BlockFunctionsV1 {
headerSize := math.Max(functionsBlockHeaderMinSize, h.BlockHeaderSize)
headerSize := max(functionsBlockHeaderMinSize, h.BlockHeaderSize)
return newSymbolsDecoder[v1.InMemoryFunction](h, &functionsBlockDecoder{headerSize: headerSize}), nil
}
return nil, fmt.Errorf("%w: unknown functions format: %d", ErrUnknownVersion, h.Format)
Expand Down
3 changes: 1 addition & 2 deletions pkg/phlaredb/symdb/locations.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (

v1 "github.com/grafana/pyroscope/pkg/phlaredb/schemas/v1"
"github.com/grafana/pyroscope/pkg/slices"
"github.com/grafana/pyroscope/pkg/util/math"
)

const maxLocationLines = 255
Expand Down Expand Up @@ -177,7 +176,7 @@ type locationsBlockDecoder struct {

func newLocationsDecoder(h SymbolsBlockHeader) (*symbolsDecoder[v1.InMemoryLocation], error) {
if h.Format == BlockLocationsV1 {
headerSize := math.Max(locationsBlockHeaderMinSize, h.BlockHeaderSize)
headerSize := max(locationsBlockHeaderMinSize, h.BlockHeaderSize)
return newSymbolsDecoder[v1.InMemoryLocation](h, &locationsBlockDecoder{headerSize: headerSize}), nil
}
return nil, fmt.Errorf("%w: unknown locations format: %d", ErrUnknownVersion, h.Format)
Expand Down
3 changes: 1 addition & 2 deletions pkg/phlaredb/symdb/mappings.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ import (

v1 "github.com/grafana/pyroscope/pkg/phlaredb/schemas/v1"
"github.com/grafana/pyroscope/pkg/slices"
"github.com/grafana/pyroscope/pkg/util/math"
)

var (
Expand Down Expand Up @@ -184,7 +183,7 @@ type mappingsBlockDecoder struct {

func newMappingsDecoder(h SymbolsBlockHeader) (*symbolsDecoder[v1.InMemoryMapping], error) {
if h.Format == BlockMappingsV1 {
headerSize := math.Max(mappingsBlockHeaderMinSize, h.BlockHeaderSize)
headerSize := max(mappingsBlockHeaderMinSize, h.BlockHeaderSize)
return newSymbolsDecoder[v1.InMemoryMapping](h, &mappingsBlockDecoder{headerSize: headerSize}), nil
}
return nil, fmt.Errorf("%w: unknown mappings format: %d", ErrUnknownVersion, h.Format)
Expand Down
4 changes: 1 addition & 3 deletions pkg/phlaredb/symdb/stacktrace_tree.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ import (
"unsafe"

"github.com/dgryski/go-groupvarint"

"github.com/grafana/pyroscope/pkg/util/math"
)

const (
Expand Down Expand Up @@ -321,7 +319,7 @@ func (d *treeDecoder) unmarshal(t *parentPointerTree, r io.Reader) error {
// group buffer, whichever is smaller.
xn := len(t.nodes) - np // remaining nodes
// Note that g should always be a multiple of 4.
g = g[:math.Min((xn+xn%2)*2, d.groupBuffer)]
g = g[:min((xn+xn%2)*2, d.groupBuffer)]
if len(g)%4 != 0 {
return io.ErrUnexpectedEOF
}
Expand Down
3 changes: 1 addition & 2 deletions pkg/phlaredb/symdb/strings.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (
"unsafe"

"github.com/grafana/pyroscope/pkg/slices"
"github.com/grafana/pyroscope/pkg/util/math"
)

const maxStringLen = 1<<16 - 1
Expand Down Expand Up @@ -120,7 +119,7 @@ type stringsBlockDecoder struct {

func newStringsDecoder(h SymbolsBlockHeader) (*symbolsDecoder[string], error) {
if h.Format == BlockStringsV1 {
headerSize := math.Max(stringsBlockHeaderMinSize, h.BlockHeaderSize)
headerSize := max(stringsBlockHeaderMinSize, h.BlockHeaderSize)
return newSymbolsDecoder[string](h, &stringsBlockDecoder{headerSize: headerSize}), nil
}
return nil, fmt.Errorf("%w: unknown strings format: %d", ErrUnknownVersion, h.Format)
Expand Down
5 changes: 2 additions & 3 deletions pkg/querier/querier.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ import (
"github.com/grafana/pyroscope/pkg/phlaredb/bucketindex"
"github.com/grafana/pyroscope/pkg/pprof"
"github.com/grafana/pyroscope/pkg/storegateway"
pmath "github.com/grafana/pyroscope/pkg/util/math"
"github.com/grafana/pyroscope/pkg/util/spanlogger"
"github.com/grafana/pyroscope/pkg/validation"
)
Expand Down Expand Up @@ -826,10 +825,10 @@ func splitQueryToStores(start, end model.Time, now model.Time, queryStoreAfter t
queries.queryStoreAfter = queryStoreAfter
cutOff := now.Add(-queryStoreAfter)
if start.Before(cutOff) {
queries.storeGateway = storeQuery{shouldQuery: true, start: start, end: pmath.Min(cutOff, end)}
queries.storeGateway = storeQuery{shouldQuery: true, start: start, end: min(cutOff, end)}
}
if end.After(cutOff) {
queries.ingester = storeQuery{shouldQuery: true, start: pmath.Max(cutOff, start), end: end}
queries.ingester = storeQuery{shouldQuery: true, start: max(cutOff, start), end: end}
// Note that the ranges must not overlap.
if queries.storeGateway.shouldQuery {
queries.ingester.start++
Expand Down
19 changes: 0 additions & 19 deletions pkg/util/math/math.go

This file was deleted.

0 comments on commit b5d11eb

Please sign in to comment.