Skip to content

Commit

Permalink
json: configurable numeric decoding (#137)
Browse files Browse the repository at this point in the history
* json: stylistic improvements, better code reuse

Initial benchmark results show this change
to be approximately performance-neutral.

* bump tested Go versions to just 1.20 and 1.21
* json: add ParseFlags values UseInt64, UseUint64, UseBigInt
* json: use atomic.Pointer
  • Loading branch information
extemporalgenome authored Dec 4, 2023
1 parent 6dfc1b0 commit 3055897
Show file tree
Hide file tree
Showing 13 changed files with 334 additions and 111 deletions.
88 changes: 45 additions & 43 deletions .github/workflows/benchmark.yml
Original file line number Diff line number Diff line change
@@ -1,61 +1,63 @@
---
name: Benchmark

on:
- pull_request
"on":
- pull_request

jobs:
benchmark:
strategy:
matrix:
ref:
- master
- ${{ github.sha }}
- master
- ${{ github.sha }}

runs-on: ubuntu-latest

steps:
- name: Steup Go
uses: actions/setup-go@v2
with:
go-version: 1.17

- name: Checkout
uses: actions/checkout@v2
with:
ref: ${{ matrix.ref }}

- name: Run Benchmarks
run: go test -v -run '^$' -bench '(Marshal|Unmarshal)$/codeResponse' -benchmem -benchtime 3s -cpu 1 -count 5 ./json | tee bench.txt

- name: Upload Benchmarks
uses: actions/upload-artifact@v2
with:
name: ${{ matrix.ref }}
path: bench.txt
- name: Setup Go
uses: actions/setup-go@v2
with:
go-version: "1.21"

- name: Checkout
uses: actions/checkout@v2
with:
ref: ${{ matrix.ref }}

- name: Run Benchmarks
# Without 6 iterations, benchstat will claim statistical insignificance.
run: go test -v -run '^$' -bench '(Marshal|Unmarshal)$/codeResponse' -benchmem -benchtime 3s -cpu 1 -count 6 ./json | tee bench.txt

- name: Upload Benchmarks
uses: actions/upload-artifact@v2
with:
name: ${{ matrix.ref }}
path: bench.txt

benchstat:
needs: [benchmark]
runs-on: ubuntu-latest

steps:
- name: Steup Go
uses: actions/setup-go@v2
with:
go-version: 1.17

- name: Setup Benchstat
run: go install golang.org/x/perf/cmd/benchstat@latest

- name: Download Benchmark Results
uses: actions/download-artifact@v2
with:
path: .

- name: Run Benchstat
run: benchstat ./master/bench.txt ./${{ github.sha }}/bench.txt | tee benchstat.txt

- name: Upload Benchstat Results
uses: actions/upload-artifact@v2
with:
name: benchstat
path: benchstat.txt
- name: Steup Go
uses: actions/setup-go@v2
with:
go-version: "1.21"

- name: Setup Benchstat
run: go install golang.org/x/perf/cmd/benchstat@latest

- name: Download Benchmark Results
uses: actions/download-artifact@v2
with:
path: .

- name: Run Benchstat
run: benchstat ./master/bench.txt ./${{ github.sha }}/bench.txt | tee benchstat.txt

- name: Upload Benchstat Results
uses: actions/upload-artifact@v2
with:
name: benchstat
path: benchstat.txt
29 changes: 14 additions & 15 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
@@ -1,30 +1,29 @@
---
name: Test

on:
- pull_request
"on":
- pull_request

jobs:
test:
strategy:
matrix:
go:
- 1.14
- 1.15
- 1.16
- 1.17
- "1.20"
- "1.21"

runs-on: ubuntu-latest

steps:
- uses: actions/checkout@v2
- uses: actions/checkout@v2

- name: Setup Go ${{ matrix.go }}
uses: actions/setup-go@v2
with:
go-version: ${{ matrix.go }}
- name: Setup Go ${{ matrix.go }}
uses: actions/setup-go@v2
with:
go-version: ${{ matrix.go }}

- name: Download Dependencies
run: go mod download
- name: Download Dependencies
run: go mod download

- name: Run Tests
run: make test
- name: Run Tests
run: make test
7 changes: 2 additions & 5 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
.PHONY: test bench-simple clean update-golang-test fuzz fuzz-json

golang.version ?= 1.15.2
golang.version ?= 1.21
golang.tmp.root := /tmp/golang$(golang.version)
golang.tmp.json.root := $(golang.tmp.root)/go-go$(golang.version)/src/encoding/json
golang.test.files := $(wildcard json/golang_*_test.go)
Expand All @@ -10,7 +10,7 @@ go-fuzz-build := ${GOPATH}/bin/go-fuzz-build
go-fuzz-corpus := ${GOPATH}/src/github.com/dvyukov/go-fuzz-corpus
go-fuzz-dep := ${GOPATH}/src/github.com/dvyukov/go-fuzz/go-fuzz-dep

test: test-ascii test-json test-json-bugs test-json-1.17 test-proto test-iso8601 test-thrift test-purego
test: test-ascii test-json test-json-bugs test-proto test-iso8601 test-thrift test-purego

test-ascii:
go test -cover -race ./ascii
Expand All @@ -21,9 +21,6 @@ test-json:
test-json-bugs:
go test -race ./json/bugs/...

test-json-1.17:
go test -cover -race -tags go1.17 ./json

test-proto:
go test -cover -race ./proto

Expand Down
4 changes: 3 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
module github.com/segmentio/encoding

go 1.14
go 1.18

require github.com/segmentio/asm v1.1.3

require golang.org/x/sys v0.0.0-20211110154304-99a53858aa08 // indirect
31 changes: 19 additions & 12 deletions json/codec.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"encoding"
"encoding/json"
"fmt"
"math/big"
"reflect"
"sort"
"strconv"
Expand Down Expand Up @@ -49,19 +50,21 @@ type decodeFunc func(decoder, []byte, unsafe.Pointer) ([]byte, error)
type emptyFunc func(unsafe.Pointer) bool
type sortFunc func([]reflect.Value)

var (
// Eventually consistent cache mapping go types to dynamically generated
// codecs.
//
// Note: using a uintptr as key instead of reflect.Type shaved ~15ns off of
// the ~30ns Marhsal/Unmarshal functions which were dominated by the map
// lookup time for simple types like bool, int, etc..
cache unsafe.Pointer // map[unsafe.Pointer]codec
)
// Eventually consistent cache mapping go types to dynamically generated
// codecs.
//
// Note: using a uintptr as key instead of reflect.Type shaved ~15ns off of
// the ~30ns Marhsal/Unmarshal functions which were dominated by the map
// lookup time for simple types like bool, int, etc..
var cache atomic.Pointer[map[unsafe.Pointer]codec]

func cacheLoad() map[unsafe.Pointer]codec {
p := atomic.LoadPointer(&cache)
return *(*map[unsafe.Pointer]codec)(unsafe.Pointer(&p))
p := cache.Load()
if p == nil {
return nil
}

return *p
}

func cacheStore(typ reflect.Type, cod codec, oldCodecs map[unsafe.Pointer]codec) {
Expand All @@ -72,7 +75,7 @@ func cacheStore(typ reflect.Type, cod codec, oldCodecs map[unsafe.Pointer]codec)
newCodecs[t] = c
}

atomic.StorePointer(&cache, *(*unsafe.Pointer)(unsafe.Pointer(&newCodecs)))
cache.Store(&newCodecs)
}

func typeid(t reflect.Type) unsafe.Pointer {
Expand Down Expand Up @@ -838,6 +841,7 @@ func constructInlineValueEncodeFunc(encode encodeFunc) encodeFunc {
// compiles down to zero instructions.
// USE CAREFULLY!
// This was copied from the runtime; see issues 23382 and 7921.
//
//go:nosplit
func noescape(p unsafe.Pointer) unsafe.Pointer {
x := uintptr(p)
Expand Down Expand Up @@ -1078,6 +1082,7 @@ var (
float32Type = reflect.TypeOf(float32(0))
float64Type = reflect.TypeOf(float64(0))

bigIntType = reflect.TypeOf(new(big.Int))
numberType = reflect.TypeOf(json.Number(""))
stringType = reflect.TypeOf("")
stringsType = reflect.TypeOf([]string(nil))
Expand All @@ -1104,6 +1109,8 @@ var (
jsonUnmarshalerType = reflect.TypeOf((*Unmarshaler)(nil)).Elem()
textMarshalerType = reflect.TypeOf((*encoding.TextMarshaler)(nil)).Elem()
textUnmarshalerType = reflect.TypeOf((*encoding.TextUnmarshaler)(nil)).Elem()

bigIntDecoder = constructJSONUnmarshalerDecodeFunc(bigIntType, false)
)

// =============================================================================
Expand Down
Loading

0 comments on commit 3055897

Please sign in to comment.