From 5a4afcf045331c6864902e848ededc1562d5fa53 Mon Sep 17 00:00:00 2001 From: aimuz Date: Wed, 8 May 2024 21:27:13 +0800 Subject: [PATCH] fmt, fmtsort: refactor SortedMap to use slice of structs for map sorting MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change refactors the SortedMap type in the fmtsort package from using two parallel slices for keys and values to a single slice of structs. This improves code clarity and reduces the complexity of handling map entries. Affected files and their respective functions have been updated to work with the new structure, including adjustments in fmt/print.go and text/template/exec.go to iterate over the new map representation. goos: darwin goarch: arm64 pkg: fmt cpu: Apple M2 Max │ old.txt │ new.txt │ │ sec/op │ sec/op vs base │ SprintfPadding-12 22.90n ± 14% 22.49n ± 5% ~ (p=0.481 n=10) SprintfEmpty-12 3.448n ± 8% 3.070n ± 11% -10.98% (p=0.029 n=10) SprintfString-12 7.635n ± 23% 7.567n ± 22% ~ (p=0.529 n=10) SprintfTruncateString-12 14.81n ± 8% 15.32n ± 9% ~ (p=0.315 n=10) SprintfTruncateBytes-12 14.19n ± 13% 15.72n ± 11% ~ (p=0.089 n=10) SprintfSlowParsingPath-12 9.048n ± 14% 9.271n ± 16% ~ (p=0.971 n=10) SprintfQuoteString-12 42.24n ± 6% 41.38n ± 5% ~ (p=0.143 n=10) SprintfInt-12 6.727n ± 16% 6.451n ± 14% ~ (p=0.239 n=10) SprintfIntInt-12 10.92n ± 15% 10.37n ± 10% ~ (p=0.063 n=10) SprintfPrefixedInt-12 31.47n ± 4% 30.71n ± 5% ~ (p=0.404 n=10) SprintfFloat-12 13.69n ± 10% 14.88n ± 11% ~ (p=0.171 n=10) SprintfComplex-12 51.06n ± 11% 50.50n ± 5% ~ (p=0.280 n=10) SprintfBoolean-12 7.432n ± 18% 7.566n ± 19% ~ (p=0.436 n=10) SprintfHexString-12 36.58n ± 14% 35.70n ± 2% ~ (p=0.072 n=10) SprintfHexBytes-12 48.01n ± 3% 47.04n ± 3% ~ (p=0.051 n=10) SprintfBytes-12 63.90n ± 7% 61.83n ± 3% ~ (p=0.123 n=10) SprintfStringer-12 37.36n ± 9% 39.58n ± 11% ~ (p=0.529 n=10) SprintfStructure-12 162.6n ± 3% 135.8n ± 5% -16.51% (p=0.000 n=10) ManyArgs-12 35.49n ± 18% 31.10n ± 31% ~ (p=0.971 n=10) FprintInt-12 33.48n ± 3% 33.30n ± 1% -0.54% (p=0.014 n=10) FprintfBytes-12 48.36n ± 1% 49.13n ± 2% +1.59% (p=0.002 n=10) FprintIntNoAlloc-12 33.44n ± 0% 33.24n ± 1% ~ (p=0.280 n=10) ScanInts-12 130.8µ ± 1% 132.4µ ± 2% +1.22% (p=0.000 n=10) ScanRecursiveInt-12 40.16m ± 2% 40.52m ± 2% ~ (p=0.052 n=10) ScanRecursiveIntReaderWrapper-12 40.15m ± 1% 41.22m ± 1% +2.68% (p=0.000 n=10) geomean 101.9n 100.7n -1.24% │ old.txt │ new.txt │ │ B/op │ B/op vs base │ SprintfPadding-12 16.00 ± 0% 16.00 ± 0% ~ (p=1.000 n=10) ¹ SprintfEmpty-12 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfString-12 5.000 ± 0% 5.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfTruncateString-12 16.00 ± 0% 16.00 ± 0% ~ (p=1.000 n=10) ¹ SprintfTruncateBytes-12 16.00 ± 0% 16.00 ± 0% ~ (p=1.000 n=10) ¹ SprintfSlowParsingPath-12 5.000 ± 0% 5.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfQuoteString-12 32.00 ± 0% 32.00 ± 0% ~ (p=1.000 n=10) ¹ SprintfInt-12 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfIntInt-12 3.000 ± 0% 3.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfPrefixedInt-12 64.00 ± 0% 64.00 ± 0% ~ (p=1.000 n=10) ¹ SprintfFloat-12 8.000 ± 0% 8.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfComplex-12 24.00 ± 0% 24.00 ± 0% ~ (p=1.000 n=10) ¹ SprintfBoolean-12 4.000 ± 0% 4.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfHexString-12 80.00 ± 0% 80.00 ± 0% ~ (p=1.000 n=10) ¹ SprintfHexBytes-12 104.0 ± 0% 104.0 ± 0% ~ (p=1.000 n=10) ¹ SprintfBytes-12 88.00 ± 0% 88.00 ± 0% ~ (p=1.000 n=10) ¹ SprintfStringer-12 32.00 ± 0% 32.00 ± 0% ~ (p=1.000 n=10) ¹ SprintfStructure-12 216.0 ± 0% 168.0 ± 0% -22.22% (p=0.000 n=10) ManyArgs-12 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ FprintInt-12 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ FprintfBytes-12 24.00 ± 0% 24.00 ± 0% ~ (p=1.000 n=10) ¹ FprintIntNoAlloc-12 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ ScanInts-12 14.87Ki ± 0% 14.87Ki ± 0% ~ (p=1.000 n=10) ¹ ScanRecursiveInt-12 16.32Ki ± 7% 16.94Ki ± 4% ~ (p=0.441 n=10) ScanRecursiveIntReaderWrapper-12 16.33Ki ± 1% 16.35Ki ± 8% ~ (p=0.068 n=10) geomean ² -0.84% ² ¹ all samples are equal ² summaries must be >0 to compute geomean │ old.txt │ new.txt │ │ allocs/op │ allocs/op vs base │ SprintfPadding-12 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfEmpty-12 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfString-12 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfTruncateString-12 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfTruncateBytes-12 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfSlowParsingPath-12 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfQuoteString-12 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfInt-12 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfIntInt-12 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfPrefixedInt-12 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfFloat-12 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfComplex-12 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfBoolean-12 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfHexString-12 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfHexBytes-12 2.000 ± 0% 2.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfBytes-12 2.000 ± 0% 2.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfStringer-12 4.000 ± 0% 4.000 ± 0% ~ (p=1.000 n=10) ¹ SprintfStructure-12 8.000 ± 0% 6.000 ± 0% -25.00% (p=0.000 n=10) ManyArgs-12 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ FprintInt-12 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ FprintfBytes-12 1.000 ± 0% 1.000 ± 0% ~ (p=1.000 n=10) ¹ FprintIntNoAlloc-12 0.000 ± 0% 0.000 ± 0% ~ (p=1.000 n=10) ¹ ScanInts-12 1.590k ± 0% 1.590k ± 0% ~ (p=1.000 n=10) ¹ ScanRecursiveInt-12 1.592k ± 0% 1.593k ± 0% ~ (p=0.650 n=10) ScanRecursiveIntReaderWrapper-12 1.594k ± 0% 1.594k ± 0% ~ (p=0.582 n=10) geomean ² -1.14% ² ¹ all samples are equal ² summaries must be >0 to compute geomean --- src/fmt/print.go | 6 ++--- src/internal/fmtsort/sort.go | 37 +++++++++++++------------------ src/internal/fmtsort/sort_test.go | 6 ++--- src/text/template/exec.go | 4 ++-- 4 files changed, 23 insertions(+), 30 deletions(-) diff --git a/src/fmt/print.go b/src/fmt/print.go index 8d6d961228..f9f200499d 100644 --- a/src/fmt/print.go +++ b/src/fmt/print.go @@ -814,7 +814,7 @@ func (p *pp) printValue(value reflect.Value, verb rune, depth int) { p.buf.writeString(mapString) } sorted := fmtsort.Sort(f) - for i, key := range sorted.Key { + for i, m := range sorted { if i > 0 { if p.fmt.sharpV { p.buf.writeString(commaSpaceString) @@ -822,9 +822,9 @@ func (p *pp) printValue(value reflect.Value, verb rune, depth int) { p.buf.writeByte(' ') } } - p.printValue(key, verb, depth+1) + p.printValue(m.Key, verb, depth+1) p.buf.writeByte(':') - p.printValue(sorted.Value[i], verb, depth+1) + p.printValue(m.Value, verb, depth+1) } if p.fmt.sharpV { p.buf.writeByte('}') diff --git a/src/internal/fmtsort/sort.go b/src/internal/fmtsort/sort.go index 278a89bd75..ea042e1811 100644 --- a/src/internal/fmtsort/sort.go +++ b/src/internal/fmtsort/sort.go @@ -10,24 +10,21 @@ package fmtsort import ( "reflect" - "sort" + "slices" ) // Note: Throughout this package we avoid calling reflect.Value.Interface as // it is not always legal to do so and it's easier to avoid the issue than to face it. -// SortedMap represents a map's keys and values. The keys and values are -// aligned in index order: Value[i] is the value in the map corresponding to Key[i]. -type SortedMap struct { - Key []reflect.Value - Value []reflect.Value -} +// SortedMap is a slice of KeyValue pairs that simplifies sorting +// and iterating over map entries. +// +// Each KeyValue pair contains a map key and its corresponding value. +type SortedMap []KeyValue -func (o *SortedMap) Len() int { return len(o.Key) } -func (o *SortedMap) Less(i, j int) bool { return compare(o.Key[i], o.Key[j]) < 0 } -func (o *SortedMap) Swap(i, j int) { - o.Key[i], o.Key[j] = o.Key[j], o.Key[i] - o.Value[i], o.Value[j] = o.Value[j], o.Value[i] +// KeyValue holds a single key and value pair found in a map. +type KeyValue struct { + Key, Value reflect.Value } // Sort accepts a map and returns a SortedMap that has the same keys and @@ -48,7 +45,7 @@ func (o *SortedMap) Swap(i, j int) { // Otherwise identical arrays compare by length. // - interface values compare first by reflect.Type describing the concrete type // and then by concrete value as described in the previous rules. -func Sort(mapValue reflect.Value) *SortedMap { +func Sort(mapValue reflect.Value) SortedMap { if mapValue.Type().Kind() != reflect.Map { return nil } @@ -56,18 +53,14 @@ func Sort(mapValue reflect.Value) *SortedMap { // of a concurrent map update. The runtime is responsible for // yelling loudly if that happens. See issue 33275. n := mapValue.Len() - key := make([]reflect.Value, 0, n) - value := make([]reflect.Value, 0, n) + sorted := make(SortedMap, 0, n) iter := mapValue.MapRange() for iter.Next() { - key = append(key, iter.Key()) - value = append(value, iter.Value()) + sorted = append(sorted, KeyValue{iter.Key(), iter.Value()}) } - sorted := &SortedMap{ - Key: key, - Value: value, - } - sort.Stable(sorted) + slices.SortStableFunc(sorted, func(a, b KeyValue) int { + return compare(a.Key, b.Key) + }) return sorted } diff --git a/src/internal/fmtsort/sort_test.go b/src/internal/fmtsort/sort_test.go index 7d5de9f56b..29a9c2c43f 100644 --- a/src/internal/fmtsort/sort_test.go +++ b/src/internal/fmtsort/sort_test.go @@ -142,13 +142,13 @@ func sprint(data any) string { return "nil" } b := new(strings.Builder) - for i, key := range om.Key { + for i, m := range om { if i > 0 { b.WriteRune(' ') } - b.WriteString(sprintKey(key)) + b.WriteString(sprintKey(m.Key)) b.WriteRune(':') - fmt.Fprint(b, om.Value[i]) + fmt.Fprint(b, m.Value) } return b.String() } diff --git a/src/text/template/exec.go b/src/text/template/exec.go index 4c899b1c79..1a8f2fa0df 100644 --- a/src/text/template/exec.go +++ b/src/text/template/exec.go @@ -408,8 +408,8 @@ func (s *state) walkRange(dot reflect.Value, r *parse.RangeNode) { break } om := fmtsort.Sort(val) - for i, key := range om.Key { - oneIteration(key, om.Value[i]) + for _, m := range om { + oneIteration(m.Key, m.Value) } return case reflect.Chan: