Skip to content

Commit

Permalink
encoding/hex: change lookup table from string to array
Browse files Browse the repository at this point in the history 
name            old time/op  new time/op  delta
Encode/256-4     431ns ± 2%   391ns ± 2%   -9.36%  (p=0.000 n=8+8)
Encode/1024-4   1.68µs ± 0%  1.51µs ± 0%   -9.91%  (p=0.001 n=7+7)
Encode/4096-4   6.68µs ± 0%  6.03µs ± 1%   -9.69%  (p=0.000 n=8+8)
Encode/16384-4  27.0µs ± 1%  24.0µs ± 0%  -11.03%  (p=0.000 n=8+7)

Change-Id: I6994e02f77797349c4e188377d84f97dffe98399
Reviewed-on: https://go-review.googlesource.com/27254
Reviewed-by: Brad Fitzpatrick <[email protected]>
Run-TryBot: Brad Fitzpatrick <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
  • Loading branch information
@ALTree @bradfitz
ALTree authored and bradfitz committed Aug 17, 2016
1 parent 17eee31 commit 57370a8
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 1 deletion.
5 changes: 4 additions & 1 deletion src/encoding/hex/hex.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,10 @@ import (
"io"
)

const hextable = "0123456789abcdef"
var hextable = [16]byte{
'0', '1', '2', '3', '4', '5', '6', '7', '8', '9',
'a', 'b', 'c', 'd', 'e', 'f',
}

// EncodedLen returns the length of an encoding of n source bytes.
func EncodedLen(n int) int { return n * 2 }
Expand Down
16 changes: 16 additions & 0 deletions src/encoding/hex/hex_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ package hex

import (
"bytes"
"fmt"
"testing"
)

Expand Down Expand Up @@ -151,3 +152,18 @@ var expectedHexDump = []byte(`00000000 1e 1f 20 21 22 23 24 25 26 27 28 29 2a
00000010 2e 2f 30 31 32 33 34 35 36 37 38 39 3a 3b 3c 3d |./0123456789:;<=|
00000020 3e 3f 40 41 42 43 44 45 |>?@ABCDE|
`)

var sink []byte

func BenchmarkEncode(b *testing.B) {
for _, size := range []int{256, 1024, 4096, 16384} {
src := bytes.Repeat([]byte{2, 3, 5, 7, 9, 11, 13, 17}, size/8)
sink = make([]byte, 2*size)

b.Run(fmt.Sprintf("%v", size), func(b *testing.B) {
for i := 0; i < b.N; i++ {
Encode(sink, src)
}
})
}
}

2 comments on commit 57370a8

@Tasssadar
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was wondering why is there 10% difference in this case, so for anyone interested, here are the relevant assembler outputs generated for Encode function with go1.7, after ssa, from the GOSSAFUNC= output:

With string:

    MOVQ    "".src+32(FP), AX
    MOVQ    "".dst+8(FP), CX
    MOVQ    "".dst(FP), DX
    MOVQ    "".src+24(FP), BX
    MOVQ    $0, SI
    CMPQ    SI, AX
    JGE     $0, 65
    MOVBLZX (BX), DI
    MOVL    DI, R8
    SHRB    $4, DI
    ANDL    $255, DI
    LEAQ    go.string."0123456789abcdef"(SB), R9
    MOVBLZX (R9)(DI*1), DI
    MOVQ    SI, R9
    SHLQ    $1, SI
    CMPQ    SI, CX
    JCC     $0, 63
    MOVB    DI, (DX)(SI*1)
    ANDL    $15, R8
    LEAQ    go.string."0123456789abcdef"(SB), DI
    MOVBLZX (DI)(R8*1), DI
    LEAQ    1(SI), R8
    CMPQ    R8, CX
    JCC     $0, 61
    MOVB    DI, 1(DX)(SI*1)
    INCQ    BX
    LEAQ    1(R9), SI

With []byte:

    MOVQ    "".src+32(FP), AX
    LEAQ    "".hextable(SB), CX
    MOVQ    "".dst+8(FP), DX
    MOVQ    "".dst(FP), BX
    MOVQ    "".src+24(FP), SI
    MOVQ    $0, DI
    CMPQ    DI, AX
    JGE     $0, 64
    MOVBLZX (SI), R8
    MOVL    R8, R9
    SHRB    $4, R8
    ANDL    $255, R8
    MOVBLZX (CX)(R8*1), R8
    MOVQ    DI, R10
    SHLQ    $1, DI
    CMPQ    DI, DX
    JCC     $0, 62
    MOVB    R8, (BX)(DI*1)
    ANDL    $15, R9
    MOVBLZX (CX)(R9*1), R8
    LEAQ    1(DI), R9
    CMPQ    R9, DX
    JCC     $0, 60
    MOVB    R8, 1(BX)(DI*1)
    INCQ    SI
    LEAQ    1(R10), DI

The only difference I can see is the variant with string loads the pointer to string twice every iteration of Encode, whereas the []byte variant loads it only once, at the top of the function (but I'm not very good at reading assembly, feel free to correct me). Shouldn't this be optimized the same way, since both the string and []byte variant are constants?

@bradfitz
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Tasssadar, nobody on the Go team uses Github for comments on code (since we use Gerrit for code review). We only use Github for the bug tracker.

Please file a compiler optimization bug. Maybe you can include a minimal repro demo too with Benchmark functions.

Please sign in to comment.