From f421ee85307920a34b6333cbc3271f969e3716ab Mon Sep 17 00:00:00 2001 From: Yi Duan Date: Tue, 13 Dec 2022 14:26:58 +0800 Subject: [PATCH] fix:(encoder) pass `pv` through compiler recursively (#336) * pass pv * test reflect indirect * fix: pass `pv` throught compiler recursively --- decoder/decoder.go | 2 +- decoder/pools.go | 2 +- encoder/assembler_amd64_go116.go | 8 +++-- encoder/assembler_amd64_go117.go | 8 +++-- encoder/assembler_test.go | 4 +-- encoder/compiler.go | 39 +++++++++++++++----- encoder/compiler_test.go | 28 ++++++++++++--- encoder/encoder.go | 45 +++-------------------- encoder/pools.go | 50 +++++++++++++++++++++++--- encoder/primitives.go | 4 +-- internal/caching/pcache.go | 4 +-- internal/caching/pcache_test.go | 2 +- internal/rt/fastvalue.go | 4 +++ issue_test/issuex_test.go | 61 ++++++++++++++++++++++++++++++++ 14 files changed, 191 insertions(+), 70 deletions(-) create mode 100644 issue_test/issuex_test.go diff --git a/decoder/decoder.go b/decoder/decoder.go index 5ff9223..c81940b 100644 --- a/decoder/decoder.go +++ b/decoder/decoder.go @@ -179,7 +179,7 @@ func Pretouch(vt reflect.Type, opts ...option.CompileOption) error { func pretouchType(_vt reflect.Type, opts option.CompileOptions) (map[reflect.Type]bool, error) { /* compile function */ compiler := newCompiler().apply(opts) - decoder := func(vt *rt.GoType) (interface{}, error) { + decoder := func(vt *rt.GoType, _ ...interface{}) (interface{}, error) { if pp, err := compiler.compile(_vt); err != nil { return nil, err } else { diff --git a/decoder/pools.go b/decoder/pools.go index a11bced..781efac 100644 --- a/decoder/pools.go +++ b/decoder/pools.go @@ -147,7 +147,7 @@ func referenceFields(v *caching.FieldMap) int64 { return int64(uintptr(unsafe.Pointer(v))) } -func makeDecoder(vt *rt.GoType) (interface{}, error) { +func makeDecoder(vt *rt.GoType, _ ...interface{}) (interface{}, error) { if pp, err := newCompiler().compile(vt.Pack()); err != nil { return nil, err } else { diff --git a/encoder/assembler_amd64_go116.go b/encoder/assembler_amd64_go116.go index 37d6562..fbfa470 100644 --- a/encoder/assembler_amd64_go116.go +++ b/encoder/assembler_amd64_go116.go @@ -1011,11 +1011,12 @@ func (self *_Assembler) _asm_OP_drop_2(_ *_Instr) { func (self *_Assembler) _asm_OP_recurse(p *_Instr) { self.prep_buffer() // MOVE {buf}, (SP) - self.Emit("MOVQ", jit.Type(p.vt()), _AX) // MOVQ $(type(p.vt())), AX + vt, pv := p.vp() + self.Emit("MOVQ", jit.Type(vt), _AX) // MOVQ $(type(p.vt())), AX self.Emit("MOVQ", _AX, jit.Ptr(_SP, 8)) // MOVQ AX, 8(SP) /* check for indirection */ - if (p.vf() & rt.F_direct) != 0 { + if !rt.UnpackType(vt).Indirect() { self.Emit("MOVQ", _SP_p, _AX) // MOVQ SP.p, AX } else { self.Emit("MOVQ", _SP_p, _VAR_vp) // MOVQ SP.p, 48(SP) @@ -1026,6 +1027,9 @@ func (self *_Assembler) _asm_OP_recurse(p *_Instr) { self.Emit("MOVQ" , _AX, jit.Ptr(_SP, 16)) // MOVQ AX, 16(SP) self.Emit("MOVQ" , _ST, jit.Ptr(_SP, 24)) // MOVQ ST, 24(SP) self.Emit("MOVQ" , _ARG_fv, _AX) // MOVQ fv, AX + if pv { + self.Emit("BTCQ", jit.Imm(bitPointerValue), _AX) // BTCQ $1, AX + } self.Emit("MOVQ" , _AX, jit.Ptr(_SP, 32)) // MOVQ AX, 32(SP) self.call_encoder(_F_encodeTypedPointer) // CALL encodeTypedPointer self.Emit("MOVQ" , jit.Ptr(_SP, 40), _ET) // MOVQ 40(SP), ET diff --git a/encoder/assembler_amd64_go117.go b/encoder/assembler_amd64_go117.go index 3a014d5..217a5d2 100644 --- a/encoder/assembler_amd64_go117.go +++ b/encoder/assembler_amd64_go117.go @@ -1015,10 +1015,11 @@ func (self *_Assembler) _asm_OP_drop_2(_ *_Instr) { func (self *_Assembler) _asm_OP_recurse(p *_Instr) { self.prep_buffer_AX() // MOVE {buf}, (SP) - self.Emit("MOVQ", jit.Type(p.vt()), _BX) // MOVQ $(type(p.vt())), BX + vt, pv := p.vp() + self.Emit("MOVQ", jit.Type(vt), _BX) // MOVQ $(type(p.vt())), BX /* check for indirection */ - if (p.vf() & rt.F_direct) != 0 { + if !rt.UnpackType(vt).Indirect() { self.Emit("MOVQ", _SP_p, _CX) // MOVQ SP.p, CX } else { self.Emit("MOVQ", _SP_p, _VAR_vp) // MOVQ SP.p, VAR.vp @@ -1028,6 +1029,9 @@ func (self *_Assembler) _asm_OP_recurse(p *_Instr) { /* call the encoder */ self.Emit("MOVQ" , _ST, _DI) // MOVQ ST, DI self.Emit("MOVQ" , _ARG_fv, _SI) // MOVQ $fv, SI + if pv { + self.Emit("BTCQ", jit.Imm(bitPointerValue), _SI) // BTCQ $1, SI + } self.call_encoder(_F_encodeTypedPointer) // CALL encodeTypedPointer self.Emit("TESTQ", _ET, _ET) // TESTQ ET, ET self.Sjmp("JNZ" , _LB_error) // JNZ _error diff --git a/encoder/assembler_test.go b/encoder/assembler_test.go index 772f0ff..091d7f2 100644 --- a/encoder/assembler_test.go +++ b/encoder/assembler_test.go @@ -52,7 +52,7 @@ func TestEncoderMemoryCorruption(t *testing.T) { } func TestAssembler_CompileAndLoad(t *testing.T) { - p, err := newCompiler().compile(reflect.TypeOf((*bool)(nil))) + p, err := newCompiler().compile(reflect.TypeOf((*bool)(nil)), true) assert.Nil(t, err) a := newAssembler(p) f := a.Load() @@ -127,7 +127,7 @@ type RecursiveValue struct { } func mustCompile(t interface{}) _Program { - p, err := newCompiler().compile(reflect.TypeOf(t)) + p, err := newCompiler().compile(reflect.TypeOf(t), !rt.UnpackEface(t).Type.Indirect()) if err != nil { panic(err) } diff --git a/encoder/compiler.go b/encoder/compiler.go index 238bfc1..ef2dc3c 100644 --- a/encoder/compiler.go +++ b/encoder/compiler.go @@ -217,6 +217,17 @@ func newInsVt(op _Op, vt reflect.Type) _Instr { } } +func newInsVp(op _Op, vt reflect.Type, pv bool) _Instr { + i := 0 + if pv { + i = 1 + } + return _Instr { + u: packOp(op) | rt.PackInt(i), + p: unsafe.Pointer(rt.UnpackType(vt)), + } +} + func (self _Instr) op() _Op { return _Op(self.u >> 56) } @@ -243,6 +254,10 @@ func (self _Instr) vt() reflect.Type { return (*rt.GoType)(self.p).Pack() } +func (self _Instr) vp() (vt reflect.Type, pv bool) { + return (*rt.GoType)(self.p).Pack(), rt.UnpackInt(self.u) == 1 +} + func (self _Instr) i64() int64 { return int64(self.vi()) } @@ -345,6 +360,10 @@ func (self *_Program) rtt(op _Op, vt reflect.Type) { *self = append(*self, newInsVt(op, vt)) } +func (self *_Program) vp(op _Op, vt reflect.Type, pv bool) { + *self = append(*self, newInsVp(op, vt, pv)) +} + func (self _Program) disassemble() string { nb := len(self) tab := make([]bool, nb + 1) @@ -379,21 +398,21 @@ type _Compiler struct { opts option.CompileOptions pv bool tab map[reflect.Type]bool - rec map[reflect.Type]bool + rec map[reflect.Type]uint8 } func newCompiler() *_Compiler { return &_Compiler { opts: option.DefaultCompileOptions(), tab: map[reflect.Type]bool{}, - rec: map[reflect.Type]bool{}, + rec: map[reflect.Type]uint8{}, } } func (self *_Compiler) apply(opts option.CompileOptions) *_Compiler { self.opts = opts if self.opts.RecursiveDepth > 0 { - self.rec = map[reflect.Type]bool{} + self.rec = map[reflect.Type]uint8{} } return self } @@ -408,15 +427,15 @@ func (self *_Compiler) rescue(ep *error) { } } -func (self *_Compiler) compile(vt reflect.Type) (ret _Program, err error) { +func (self *_Compiler) compile(vt reflect.Type, pv bool) (ret _Program, err error) { defer self.rescue(&err) - self.compileOne(&ret, 0, vt, false) + self.compileOne(&ret, 0, vt, pv) return } func (self *_Compiler) compileOne(p *_Program, sp int, vt reflect.Type, pv bool) { if self.tab[vt] { - p.rtt(_OP_recurse, vt) + p.vp(_OP_recurse, vt, pv) } else { self.compileRec(p, sp, vt, pv) } @@ -661,9 +680,13 @@ func (self *_Compiler) compileString(p *_Program, vt reflect.Type) { func (self *_Compiler) compileStruct(p *_Program, sp int, vt reflect.Type) { if sp >= self.opts.MaxInlineDepth || p.pc() >= _MAX_ILBUF || (sp > 0 && vt.NumField() >= _MAX_FIELDS) { - p.rtt(_OP_recurse, vt) + p.vp(_OP_recurse, vt, self.pv) if self.opts.RecursiveDepth > 0 { - self.rec[vt] = true + if self.pv { + self.rec[vt] = 1 + } else { + self.rec[vt] = 0 + } } } else { self.compileStructBody(p, sp, vt) diff --git a/encoder/compiler_test.go b/encoder/compiler_test.go index 72d4759..ddd0c59 100644 --- a/encoder/compiler_test.go +++ b/encoder/compiler_test.go @@ -17,14 +17,34 @@ package encoder import ( - `reflect` - `testing` + "reflect" + "testing" + "unsafe" - `github.com/stretchr/testify/assert` + "github.com/bytedance/sonic/internal/rt" + "github.com/stretchr/testify/assert" ) func TestCompiler_Compile(t *testing.T) { - p, err := newCompiler().compile(reflect.TypeOf(_BindingValue)) + p, err := newCompiler().compile(reflect.TypeOf(_BindingValue), false) assert.Nil(t, err) p.disassemble() } + +func TestReflectDirect(t *testing.T) { + type A struct { + A int + B int + } + var a A + var b = &a + println("b:", unsafe.Pointer(b)) + v := rt.UnpackEface(a) + vv := reflect.ValueOf(a) + _ = vv + println("v:", v.Type.KindFlags, v.Value) + pv := rt.UnpackEface(&a) + pvv := reflect.ValueOf(&a) + _ = pvv + println("pv:", pv.Type.KindFlags, pv.Value) +} \ No newline at end of file diff --git a/encoder/encoder.go b/encoder/encoder.go index 0b9fd31..19a35de 100644 --- a/encoder/encoder.go +++ b/encoder/encoder.go @@ -38,6 +38,9 @@ const ( bitCompactMarshaler bitNoQuoteTextMarshaler bitNoNullSliceOrMap + + // used for recursive compile + bitPointerValue = 63 ) const ( @@ -284,47 +287,7 @@ func Pretouch(vt reflect.Type, opts ...option.CompileOption) error { opt(&cfg) break } - return pretouchRec(map[reflect.Type]bool{vt:true}, cfg) -} - -func pretouchType(_vt reflect.Type, opts option.CompileOptions) (map[reflect.Type]bool, error) { - /* compile function */ - compiler := newCompiler().apply(opts) - encoder := func(vt *rt.GoType) (interface{}, error) { - if pp, err := compiler.compile(_vt); err != nil { - return nil, err - } else { - return newAssembler(pp).Load(), nil - } - } - - /* find or compile */ - vt := rt.UnpackType(_vt) - if val := programCache.Get(vt); val != nil { - return nil, nil - } else if _, err := programCache.Compute(vt, encoder); err == nil { - return compiler.rec, nil - } else { - return nil, err - } -} - -func pretouchRec(vtm map[reflect.Type]bool, opts option.CompileOptions) error { - if opts.RecursiveDepth < 0 || len(vtm) == 0 { - return nil - } - next := make(map[reflect.Type]bool) - for vt := range(vtm) { - sub, err := pretouchType(vt, opts) - if err != nil { - return err - } - for svt := range(sub) { - next[svt] = true - } - } - opts.RecursiveDepth -= 1 - return pretouchRec(next, opts) + return pretouchRec(map[reflect.Type]uint8{vt: 0}, cfg) } // Valid validates json and returns first non-blank character position, diff --git a/encoder/pools.go b/encoder/pools.go index 8214e1b..3f1fc22 100644 --- a/encoder/pools.go +++ b/encoder/pools.go @@ -21,8 +21,10 @@ import ( `sync` `unsafe` `errors` + `reflect` `github.com/bytedance/sonic/internal/caching` + `github.com/bytedance/sonic/option` `github.com/bytedance/sonic/internal/rt` ) @@ -129,20 +131,60 @@ func freeBuffer(p *bytes.Buffer) { bufferPool.Put(p) } -func makeEncoder(vt *rt.GoType) (interface{}, error) { - if pp, err := newCompiler().compile(vt.Pack()); err != nil { +func makeEncoder(vt *rt.GoType, ex ...interface{}) (interface{}, error) { + if pp, err := newCompiler().compile(vt.Pack(), ex[0].(bool)); err != nil { return nil, err } else { return newAssembler(pp).Load(), nil } } -func findOrCompile(vt *rt.GoType) (_Encoder, error) { +func findOrCompile(vt *rt.GoType, pv bool) (_Encoder, error) { if val := programCache.Get(vt); val != nil { return val.(_Encoder), nil - } else if ret, err := programCache.Compute(vt, makeEncoder); err == nil { + } else if ret, err := programCache.Compute(vt, makeEncoder, pv); err == nil { return ret.(_Encoder), nil } else { return nil, err } } + +func pretouchType(_vt reflect.Type, opts option.CompileOptions, v uint8) (map[reflect.Type]uint8, error) { + /* compile function */ + compiler := newCompiler().apply(opts) + encoder := func(vt *rt.GoType, ex ...interface{}) (interface{}, error) { + if pp, err := compiler.compile(_vt, ex[0].(bool)); err != nil { + return nil, err + } else { + return newAssembler(pp).Load(), nil + } + } + + /* find or compile */ + vt := rt.UnpackType(_vt) + if val := programCache.Get(vt); val != nil { + return nil, nil + } else if _, err := programCache.Compute(vt, encoder, v == 1); err == nil { + return compiler.rec, nil + } else { + return nil, err + } +} + +func pretouchRec(vtm map[reflect.Type]uint8, opts option.CompileOptions) error { + if opts.RecursiveDepth < 0 || len(vtm) == 0 { + return nil + } + next := make(map[reflect.Type]uint8) + for vt, v := range vtm { + sub, err := pretouchType(vt, opts, v) + if err != nil { + return err + } + for svt, v := range sub { + next[svt] = v + } + } + opts.RecursiveDepth -= 1 + return pretouchRec(next, opts) +} \ No newline at end of file diff --git a/encoder/primitives.go b/encoder/primitives.go index e7ab419..451b997 100644 --- a/encoder/primitives.go +++ b/encoder/primitives.go @@ -68,9 +68,9 @@ func encodeString(buf *[]byte, val string) error { func encodeTypedPointer(buf *[]byte, vt *rt.GoType, vp *unsafe.Pointer, sb *_Stack, fv uint64) error { if vt == nil { return encodeNil(buf) - } else if fn, err := findOrCompile(vt); err != nil { + } else if fn, err := findOrCompile(vt, (fv&(1<