Skip to content

Commit

Permalink
fmt, fmtsort: refactor SortedMap to use slice of structs for map sorting
Browse files Browse the repository at this point in the history
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
  • Loading branch information
aimuz committed May 9, 2024
1 parent 4ed358b commit 5a4afcf
Show file tree
Hide file tree
Showing 4 changed files with 23 additions and 30 deletions.
6 changes: 3 additions & 3 deletions src/fmt/print.go
Original file line number Diff line number Diff line change
Expand Up @@ -814,17 +814,17 @@ 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)
} else {
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('}')
Expand Down
37 changes: 15 additions & 22 deletions src/internal/fmtsort/sort.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -48,26 +45,22 @@ 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
}
// Note: this code is arranged to not panic even in the presence
// 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 := &SortedMap{
Key: key,
Value: value,
sorted = append(sorted, KeyValue{iter.Key(), iter.Value()})
}
sort.Stable(sorted)
slices.SortStableFunc(sorted, func(a, b KeyValue) int {
return compare(a.Key, b.Key)
})
return sorted
}

Expand Down
6 changes: 3 additions & 3 deletions src/internal/fmtsort/sort_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
Expand Down
4 changes: 2 additions & 2 deletions src/text/template/exec.go
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down

0 comments on commit 5a4afcf

Please sign in to comment.