Skip to content

Commit

Permalink
codec: minor cleanups prior to cutting release
Browse files Browse the repository at this point in the history
- use sync/atomic to prevent race condition within rpc test
- ensure decDriver implementations and typeInfo fit into cache lines completely
  • Loading branch information
ugorji committed Mar 30, 2021
1 parent 1768cf4 commit f7a0860
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 17 deletions.
4 changes: 4 additions & 0 deletions codec/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,10 @@ _tests() {
local echo=1
local nc=2 # count
local cpus="1,$(nproc)"
# if using the race detector, then set nc to
if [[ " ${zargs[@]} " =~ "-race" ]]; then
cpus="$(nproc)"
fi
local a=( "" "codec.notfastpath" "codec.safe" "codec.notfastpath codec.safe" "codecgen" )
local b=()
local c=()
Expand Down
22 changes: 17 additions & 5 deletions codec/codec_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,12 +249,23 @@ type TestRpcABC struct {
}

type TestRpcInt struct {
i int
i int64
}

func (r *TestRpcInt) Update(n int, res *int) error { r.i = n; *res = r.i; return nil }
func (r *TestRpcInt) Square(ignore int, res *int) error { *res = r.i * r.i; return nil }
func (r *TestRpcInt) Mult(n int, res *int) error { *res = r.i * n; return nil }
func (r *TestRpcInt) Update(n int, res *int) error {
atomic.StoreInt64(&r.i, int64(n))
*res = n
return nil
}
func (r *TestRpcInt) Square(ignore int, res *int) error {
i := int(atomic.LoadInt64(&r.i))
*res = i * i
return nil
}
func (r *TestRpcInt) Mult(n int, res *int) error {
*res = int(atomic.LoadInt64(&r.i)) * n
return nil
}
func (r *TestRpcInt) EchoStruct(arg TestRpcABC, res *string) error {
*res = fmt.Sprintf("%#v", arg)
return nil
Expand Down Expand Up @@ -475,6 +486,7 @@ func testCheckErr(t *testing.T, err error) {
}

func testCheckEqual(t *testing.T, v1 interface{}, v2 interface{}, desc string) {
t.Helper()
if err := deepEqual(v1, v2); err != nil {
t.Logf("Not Equal: %s: %v", desc, err)
if testVerbose {
Expand Down Expand Up @@ -1565,7 +1577,7 @@ func doTestCodecRpcOne(t *testing.T, rr Rpc, h Handle, doRequest bool, exitSleep
var up, sq, mult int
var rstr string
testCheckErr(t, cl.Call("TestRpcInt.Update", 5, &up))
testCheckEqual(t, testRpcInt.i, 5, "testRpcInt.i=5")
testCheckEqual(t, atomic.LoadInt64(&testRpcInt.i), int64(5), "testRpcInt.i=5")
testCheckEqual(t, up, 5, "up=5")
testCheckErr(t, cl.Call("TestRpcInt.Square", 1, &sq))
testCheckEqual(t, sq, 25, "sq=25")
Expand Down
6 changes: 3 additions & 3 deletions codec/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,9 @@ import (
const msgBadDesc = "unrecognized descriptor byte"

const (
decDefMaxDepth = 1024 // maximum depth
decDefChanCap = 64 // should be large, as cap cannot be expanded
decScratchByteArrayLen = (8 + 2) * 8 // around cacheLineSize ie ~64, depending on Decoder size
decDefMaxDepth = 1024 // maximum depth
decDefChanCap = 64 // should be large, as cap cannot be expanded
decScratchByteArrayLen = (8 + 2 + 2) * 8 // around cacheLineSize ie ~64, depending on Decoder size

// MARKER: massage decScratchByteArrayLen to ensure xxxDecDriver structs fit within cacheLine*N

Expand Down
2 changes: 1 addition & 1 deletion codec/gen.go
Original file line number Diff line number Diff line change
Expand Up @@ -2153,7 +2153,7 @@ func genNonPtr(t reflect.Type) reflect.Type {
func genFastpathUnderlying(t reflect.Type, rtid uintptr, ti *typeInfo) (tu reflect.Type, rtidu uintptr) {
tu = t
rtidu = rtid
if ti.pkgpath != "" {
if ti.flagHasPkgPath {
tu = ti.fastpathUnderlying
rtidu = rt2id(tu)
}
Expand Down
20 changes: 12 additions & 8 deletions codec/helper.go
Original file line number Diff line number Diff line change
Expand Up @@ -1183,7 +1183,7 @@ func (x *basicHandleRuntimeState) fnLoad(rt reflect.Type, rtid uintptr, tinfos *
// Consequently, the value of doing this quick allocation to elide the overhead cost of
// non-optimized (not-unsafe) reflection is a fair price.
var rtid2 uintptr
if ti.pkgpath == "" { // un-named type (slice or mpa or array)
if !ti.flagHasPkgPath { // un-named type (slice or mpa or array)
rtid2 = rtid
if rk == reflect.Array {
rtid2 = rt2id(ti.key) // ti.key for arrays = reflect.SliceOf(ti.elem)
Expand Down Expand Up @@ -1841,9 +1841,10 @@ func (p sfiSortedByEncName) Less(i, j int) bool { return p[uint(i)].encName < p[
// - If type is text(M/Unm)arshaler, call Text(M/Unm)arshal method
// - Else decode appropriately based on the reflect.Kind
type typeInfo struct {
rt reflect.Type
elem reflect.Type
pkgpath string
rt reflect.Type
elem reflect.Type

// pkgpath string

rtid uintptr

Expand Down Expand Up @@ -1887,6 +1888,7 @@ type typeInfo struct {

keykind, elemkind uint8

flagHasPkgPath bool // Type.PackagePath != ""
flagCustom bool // does this have custom implementation?
flagComparable bool
flagCanTransient bool
Expand Down Expand Up @@ -2143,8 +2145,10 @@ func (x *TypeInfos) load(rt reflect.Type) (pti *typeInfo) {
kind: uint8(rk),
size: uint32(rt.Size()),
numMeth: uint16(rt.NumMethod()),
pkgpath: rt.PkgPath(),
keyType: valueTypeString, // default it - so it's never 0

// pkgpath: rt.PkgPath(),
flagHasPkgPath: rt.PkgPath() != "",
}

// bset sets custom implementation flags
Expand Down Expand Up @@ -2228,7 +2232,7 @@ func (x *TypeInfos) load(rt reflect.Type) (pti *typeInfo) {
ti.tikey = x.get(rt2id(tt), tt)
ti.keykind = uint8(ti.key.Kind())
ti.keysize = uint32(ti.key.Size())
if ti.pkgpath != "" {
if ti.flagHasPkgPath {
ti.fastpathUnderlying = reflect.MapOf(ti.key, ti.elem)
}
case reflect.Slice:
Expand All @@ -2242,7 +2246,7 @@ func (x *TypeInfos) load(rt reflect.Type) (pti *typeInfo) {
ti.tielem = x.get(rt2id(tt), tt)
ti.elemkind = uint8(ti.elem.Kind())
ti.elemsize = uint32(ti.elem.Size())
if ti.pkgpath != "" {
if ti.flagHasPkgPath {
ti.fastpathUnderlying = reflect.SliceOf(ti.elem)
}
case reflect.Chan:
Expand All @@ -2269,7 +2273,7 @@ func (x *TypeInfos) load(rt reflect.Type) (pti *typeInfo) {
ti.key = reflect.SliceOf(ti.elem)
ti.keykind = uint8(reflect.Slice)
ti.keysize = uint32(ti.key.Size())
if ti.pkgpath != "" {
if ti.flagHasPkgPath {
ti.fastpathUnderlying = ti.key
}

Expand Down

0 comments on commit f7a0860

Please sign in to comment.