Switch separate min/max compute to single function

zix99 · Dec 17, 2024 · 6a24ced · 6a24ced
1 parent 6600e6d
commit 6a24ced
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 30 deletions.
diff --git a/pkg/aggregation/table.go b/pkg/aggregation/table.go
@@ -120,32 +120,26 @@ func (s *TableAggregator) OrderedRows(sorter sorting.NameValueSorter) []*TableRo
 	return rows
 }
 
-func (s *TableAggregator) ComputeMin() (ret int64) {
-	ret = math.MaxInt64
+func (s *TableAggregator) ComputeMinMax() (min, max int64) {
+	min, max = math.MaxInt64, math.MinInt64
+
 	for _, r := range s.rows {
 		for colKey := range s.cols {
-			if val := r.cols[colKey]; val < ret {
-				ret = val
+			val := r.cols[colKey]
+			if val < min {
+				min = val
+			}
+			if val > max {
+				max = val
 			}
 		}
 	}
-	if ret == math.MaxInt64 {
-		return 0
-	}
-	return
-}
 
-func (s *TableAggregator) ComputeMax() (ret int64) {
-	ret = math.MinInt64
-	for _, r := range s.rows {
-		for colKey := range s.cols {
-			if val := r.cols[colKey]; val > ret {
-				ret = val
-			}
-		}
+	if min == math.MinInt64 {
+		min = 0
 	}
-	if ret == math.MinInt64 {
-		return 0
+	if max == math.MaxInt64 {
+		max = 0
 	}
 	return
 }

diff --git a/pkg/aggregation/table_test.go b/pkg/aggregation/table_test.go
@@ -46,8 +46,9 @@ func TestSimpleTable(t *testing.T) {
 	assert.Equal(t, int64(5), table.Sum())
 
 	// Minmax
-	assert.Equal(t, int64(0), table.ComputeMin())
-	assert.Equal(t, int64(3), table.ComputeMax())
+	min, max := table.ComputeMinMax()
+	assert.Equal(t, int64(0), min)
+	assert.Equal(t, int64(3), max)
 }
 
 func TestTableMultiIncrement(t *testing.T) {
@@ -74,8 +75,9 @@ func TestTableMultiIncrement(t *testing.T) {
 	assert.Equal(t, int64(6), table.Sum())
 
 	// Minmax
-	assert.Equal(t, int64(0), table.ComputeMin())
-	assert.Equal(t, int64(5), table.ComputeMax())
+	min, max := table.ComputeMinMax()
+	assert.Equal(t, int64(0), min)
+	assert.Equal(t, int64(5), max)
 }
 
 func TestSingleRowTable(t *testing.T) {
@@ -116,3 +118,16 @@ func TestTrimData(t *testing.T) {
 	assert.Len(t, table.Rows(), 1)
 	assert.Len(t, table.Rows()[0].cols, 5)
 }
+
+// BenchmarkMinMax-4   	 1020728	      1234 ns/op	       0 B/op	       0 allocs/op
+func BenchmarkMinMax(b *testing.B) {
+	table := NewTable(" ")
+	for i := 0; i < 10; i++ {
+		table.Sample(fmt.Sprintf("%d a", i))
+		table.Sample(fmt.Sprintf("%d b", i))
+	}
+
+	for i := 0; i < b.N; i++ {
+		table.ComputeMinMax()
+	}
+}
diff --git a/pkg/multiterm/termrenderers/heatmap.go b/pkg/multiterm/termrenderers/heatmap.go
@@ -61,12 +61,16 @@ func (s *Heatmap) WriteFooter(idx int, line string) {
 
 func (s *Heatmap) UpdateMinMaxFromData(agg *aggregation.TableAggregator) {
 	min := s.minVal
-	if !s.FixedMin {
-		min = agg.ComputeMin()
-	}
 	max := s.maxVal
-	if !s.FixedMax {
-		max = agg.ComputeMax()
+
+	if !s.FixedMin || !s.FixedMax {
+		tableMin, tableMax := agg.ComputeMinMax()
+		if !s.FixedMin {
+			min = tableMin
+		}
+		if !s.FixedMax {
+			max = tableMax
+		}
 	}
 
 	s.UpdateMinMax(min, max)

diff --git a/pkg/multiterm/termrenderers/spark.go b/pkg/multiterm/termrenderers/spark.go
@@ -28,8 +28,7 @@ func NewSpark(term multiterm.MultilineTerm, rows, cols int) *Spark {
 }
 
 func (s *Spark) WriteTable(agg *aggregation.TableAggregator, rowSorter, colSorter sorting.NameValueSorter) {
-	minVal := agg.ComputeMin() // Optimization: ComputeMinMax()
-	maxVal := agg.ComputeMax()
+	minVal, maxVal := agg.ComputeMinMax()
 
 	colNames := agg.OrderedColumns(colSorter)
 	if len(colNames) > s.colCount {