Skip to content

Commit

Permalink
codec: Use ContainerNext everywhere CheckBreak is used, and enable mo…
Browse files Browse the repository at this point in the history
…re inlining in generated code

- remove direct calls to CheckBreak(); use ContainerNext instead.
  This cleans up the codebase dramatically, and allows us optimize ContainerNext as needed.
- updated genVersion to 26
- moved some Decoder fields from decRd into decoder
- moved some Encoder fields from encWr into Encoder
- removed genHelperEncoder.WriteStr (which wasn't inline'able) and add genHelperEncoder.EncWr()
  to give access to encWr.WriteStr directly
- Ensure WriteStr and StringZC are inline'able (to give optimal performance to codecgen)
  • Loading branch information
ugorji committed Feb 3, 2023
1 parent aadf505 commit 561b807
Show file tree
Hide file tree
Showing 18 changed files with 1,744 additions and 4,157 deletions.
2 changes: 1 addition & 1 deletion codec/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ caveats. See Encode documentation.

```go
const CborStreamBytes byte = 0x5f ...
const GenVersion = 25
const GenVersion = 26
var SelfExt = &extFailWrapper{}
var GoRpc goRpc
var MsgpackSpecRpc msgpackSpecRpc
Expand Down
12 changes: 7 additions & 5 deletions codec/build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,10 @@ _usage() {

cat <<EOF
primary usage: $0
-[tesow m n l d] -> [t=tests (e=extra, s=short, o=cover, w=wait), m=make, n=inlining diagnostics, l=mid-stack inlining, d=race detector]
-v -> v=verbose
-t[esow] -> t=tests [e=extra, s=short, o=cover, w=wait]
-[md] -> [m=make, d=race detector]
-[n l i] -> [n=inlining diagnostics, l=mid-stack inlining, i=check inlining for path (path)]
-v -> v=verbose
EOF
if [[ "$(type -t _usage_run)" = "function" ]]; then _usage_run ; fi
}
Expand All @@ -329,7 +331,7 @@ _main() {
local gocmd=${MYGOCMD:-go}

OPTIND=1
while getopts ":cetmnrgpfvldsowkxyzb:" flag
while getopts ":cetmnrgpfvldsowkxyzi" flag
do
case "x$flag" in
'xo') zcover=1 ;;
Expand All @@ -341,7 +343,7 @@ _main() {
'xl') zargs+=("-gcflags"); zargs+=("-l=4") ;;
'xn') zargs+=("-gcflags"); zargs+=("-m=2") ;;
'xd') zargs+=("-race") ;;
'xb') x='b'; zbenchflags=${OPTARG} ;;
# 'xi') x='i'; zbenchflags=${OPTARG} ;;
x\?) _usage; return 1 ;;
*) x=$flag ;;
esac
Expand All @@ -359,7 +361,7 @@ _main() {
'xy') _analyze_debug_types "$@" ;;
'xz') _analyze_do_inlining_and_more "$@" ;;
'xk') _go_compiler_validation_suite ;;
'xb') _bench "$@" ;;
'xi') _check_inlining_one "$@" ;;
esac
# unset zforce zargs zbenchflags
}
Expand Down
95 changes: 56 additions & 39 deletions codec/decode.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,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 + 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 + 1) * 8 // around cacheLineSize ie ~64, depending on Decoder size

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

Expand Down Expand Up @@ -150,13 +150,11 @@ type decDriver interface {
// If the format doesn't prefix the length, it returns containerLenUnknown.
// If the expected array was a nil in the stream, it returns containerLenNil.
ReadArrayStart() int
ReadArrayEnd()

// ReadMapStart will return the length of the array.
// If the format doesn't prefix the length, it returns containerLenUnknown.
// If the expected array was a nil in the stream, it returns containerLenNil.
ReadMapStart() int
ReadMapEnd()

reset()

Expand Down Expand Up @@ -186,6 +184,8 @@ type decDriverContainerTracker interface {
ReadArrayElem()
ReadMapElemKey()
ReadMapElemValue()
ReadArrayEnd()
ReadMapEnd()
}

type decNegintPosintFloatNumber interface {
Expand All @@ -202,11 +202,11 @@ func (x decDriverNoopNumberHelper) decFloat() (f float64, ok bool) { panic("decF

type decDriverNoopContainerReader struct{}

func (x decDriverNoopContainerReader) ReadArrayStart() (v int) { panic("ReadArrayStart unsupported") }
func (x decDriverNoopContainerReader) ReadArrayEnd() {}
func (x decDriverNoopContainerReader) ReadMapStart() (v int) { panic("ReadMapStart unsupported") }
func (x decDriverNoopContainerReader) ReadMapEnd() {}
func (x decDriverNoopContainerReader) CheckBreak() (v bool) { return }
// func (x decDriverNoopContainerReader) ReadArrayStart() (v int) { panic("ReadArrayStart unsupported") }
// func (x decDriverNoopContainerReader) ReadMapStart() (v int) { panic("ReadMapStart unsupported") }
func (x decDriverNoopContainerReader) ReadArrayEnd() {}
func (x decDriverNoopContainerReader) ReadMapEnd() {}
func (x decDriverNoopContainerReader) CheckBreak() (v bool) { return }

// DecodeOptions captures configuration options during decode.
type DecodeOptions struct {
Expand Down Expand Up @@ -729,38 +729,21 @@ func (d *Decoder) kStruct(f *codecFnInfo, rv reflect.Value) {
}
// Not much gain from doing it two ways for array.
// Arrays are not used as much for structs.
hasLen := containerLen >= 0
var checkbreak bool
tisfi := ti.sfi.source()
for j, si := range tisfi {
if hasLen {
if j == containerLen {
break
}
} else if d.checkBreak() {
checkbreak = true
break
}
hasLen := containerLen >= 0

// iterate all the items in the stream
// if mapped elem-wise to a field, handle it
// if more stream items than cap be mapped, error it
for j := 0; d.containerNext(j, containerLen, hasLen); j++ {
d.arrayElem()
d.kStructField(si, rv)
}
var proceed bool
if hasLen {
proceed = containerLen > len(tisfi)
} else {
proceed = !checkbreak
}
// if (hasLen && containerLen > len(tisfi)) || (!hasLen && !checkbreak) {
if proceed {
// read remaining values and throw away
for j := len(tisfi); ; j++ {
if !d.containerNext(j, containerLen, hasLen) {
break
}
d.arrayElem()
if j < len(tisfi) {
d.kStructField(tisfi[j], rv)
} else {
d.structFieldNotFound(j, "")
}
}

d.arrayEnd()
} else {
d.onerror(errNeedMapOrArrayDecodeToStruct)
Expand Down Expand Up @@ -1422,6 +1405,7 @@ func (d *Decoder) r() *decRd {

func (d *Decoder) init(h Handle) {
initHandle(h)
d.cbreak = d.js || d.cbor
d.bytes = true
d.err = errDecoderNotInitialized
d.h = h.getBasicHandle()
Expand Down Expand Up @@ -1948,7 +1932,34 @@ func (d *Decoder) decodeFloat32() float32 {

// MARKER: do not call mapEnd if mapStart returns containerLenNil.

// MARKER: optimize decoding since all formats do not truly support all decDriver'ish operations.
// - Read(Map|Array)Start is only supported by all formats.
// - CheckBreak is only supported by json and cbor.
// - Read(Map|Array)End is only supported by json.
// - Read(Map|Array)Elem(Kay|Value) is only supported by json.
// Honor these in the code, to reduce the number of interface calls (even if empty).

func (d *Decoder) checkBreak() (v bool) {
// MARKER: jsonDecDriver.CheckBreak() cannot be inlined (over budget inlining cost).
// Consequently, there's no benefit in incurring the cost of this wrapping function.
// It is faster to just call the interface method directly.

// if d.js {
// return d.jsondriver().CheckBreak()
// }
// if d.cbor {
// return d.cbordriver().CheckBreak()
// }

if d.cbreak {
v = d.d.CheckBreak()
}
return
}

func (d *Decoder) containerNext(j, containerLen int, hasLen bool) bool {
// MARKER: keep in sync with gen-helper.go.tmpl

// return (hasLen && j < containerLen) || !(hasLen || slh.d.checkBreak())
if hasLen {
return j < containerLen
Expand Down Expand Up @@ -1979,7 +1990,10 @@ func (d *Decoder) mapElemValue() {
}

func (d *Decoder) mapEnd() {
d.d.ReadMapEnd()
if d.js {
d.jsondriver().ReadMapEnd()
}
// d.d.ReadMapEnd()
d.depthDecr()
d.c = 0
}
Expand All @@ -2000,7 +2014,10 @@ func (d *Decoder) arrayElem() {
}

func (d *Decoder) arrayEnd() {
d.d.ReadArrayEnd()
if d.js {
d.jsondriver().ReadArrayEnd()
}
// d.d.ReadArrayEnd()
d.depthDecr()
d.c = 0
}
Expand Down
Loading

0 comments on commit 561b807

Please sign in to comment.