Browse Source

codec: make tracking of #calls only applicable to types which must Release() when done

Ugorji Nwoke 6 years ago
parent
commit
ee1426cffe
3 changed files with 68 additions and 51 deletions
  1. 27 20
      codec/decode.go
  2. 33 28
      codec/encode.go
  3. 8 3
      codec/helper.go

+ 27 - 20
codec/decode.go

@@ -553,21 +553,33 @@ type bufioDecReader struct {
 
 	// err error
 
-	_ [2]uint64 // padding
+	// Extensions can call Decode() within a current Decode() call.
+	// We need to know when the top level Decode() call returns,
+	// so we can decide whether to Release() or not.
+	calls uint16 // what depth in mustDecode are we in now.
+
+	_ [6]uint8 // padding
+
+	_ [1]uint64 // padding
 }
 
 func (z *bufioDecReader) reset(r io.Reader, bufsize int) {
 	z.ioDecReaderCommon.reset(r)
 	z.c = 0
+	z.calls = 0
 	if cap(z.buf) >= bufsize {
 		z.buf = z.buf[:0]
 	} else {
-		z.bytesBufPooler.end() // potentially return old one to pool
 		z.buf = z.bytesBufPooler.get(bufsize)[:0]
 		// z.buf = make([]byte, 0, bufsize)
 	}
 }
 
+func (z *bufioDecReader) release() {
+	z.buf = nil
+	z.bytesBufPooler.end()
+}
+
 func (z *bufioDecReader) readb(p []byte) {
 	var n = uint(copy(p, z.buf[z.c:]))
 	z.n += n
@@ -2294,12 +2306,7 @@ type Decoder struct {
 	depth    int16
 	maxdepth int16
 
-	// Extensions can call Decode() within a current Decode() call.
-	// We need to know when the top level Decode() call returns,
-	// so we can decide whether to Release() or not.
-	calls uint16 // what depth in mustDecode are we in now.
-
-	_ [2]uint8 // padding
+	_ [4]uint8 // padding
 
 	is map[string]string // used for interning strings
 
@@ -2359,7 +2366,6 @@ func (d *Decoder) resetCommon() {
 	// d.r = &d.decReaderSwitch
 	d.d.reset()
 	d.err = nil
-	d.calls = 0
 	d.depth = 0
 	d.maxdepth = d.h.MaxDepth
 	if d.maxdepth <= 0 {
@@ -2523,16 +2529,21 @@ func (d *Decoder) MustDecode(v interface{}) {
 // This provides insight to the code location that triggered the error.
 func (d *Decoder) mustDecode(v interface{}) {
 	// TODO: Top-level: ensure that v is a pointer and not nil.
-	d.calls++
 	if d.d.TryDecodeAsNil() {
 		setZero(v)
-	} else {
+		return
+	}
+	if d.bi == nil {
 		d.decode(v)
+		return
 	}
+
+	d.bi.calls++
+	d.decode(v)
 	// xprintf(">>>>>>>> >>>>>>>> num decFns: %v\n", d.cf.sn)
-	d.calls--
-	if !d.h.ExplicitRelease && d.calls == 0 {
-		d.Release()
+	d.bi.calls--
+	if !d.h.ExplicitRelease && d.bi.calls == 0 {
+		d.bi.release()
 	}
 }
 
@@ -2558,12 +2569,8 @@ func (d *Decoder) finalize() {
 //
 // By default, Release() is automatically called unless the option ExplicitRelease is set.
 func (d *Decoder) Release() {
-	if useFinalizers && removeFinalizerOnRelease {
-		runtime.SetFinalizer(d, nil)
-	}
-	if d.bi != nil && d.bi.bytesBufPooler.pool != nil {
-		d.bi.buf = nil
-		d.bi.bytesBufPooler.end()
+	if d.bi != nil {
+		d.bi.release()
 	}
 	// d.decNakedPooler.end()
 }

+ 33 - 28
codec/encode.go

@@ -279,10 +279,18 @@ type bufioEncWriter struct {
 	buf []byte
 	w   io.Writer
 	n   int
+	sz  int // buf size
+
+	// Extensions can call Encode() within a current Encode() call.
+	// We need to know when the top level Encode() call returns,
+	// so we can decide whether to Release() or not.
+	calls uint16 // what depth in mustDecode are we in now.
+
+	_ [6]uint8 // padding
 
 	bytesBufPooler
 
-	_ [3]uint64 // padding
+	_ [1]uint64 // padding
 	// a int
 	// b   [4]byte
 	// err
@@ -291,20 +299,24 @@ type bufioEncWriter struct {
 func (z *bufioEncWriter) reset(w io.Writer, bufsize int) {
 	z.w = w
 	z.n = 0
+	z.calls = 0
 	if bufsize <= 0 {
 		bufsize = defEncByteBufSize
 	}
-	if z.buf == nil {
-		z.buf = z.bytesBufPooler.get(bufsize)
-	} else if cap(z.buf) >= bufsize {
+	z.sz = bufsize
+	if cap(z.buf) >= bufsize {
 		z.buf = z.buf[:cap(z.buf)]
 	} else {
-		z.bytesBufPooler.end() // potentially return old one to pool
 		z.buf = z.bytesBufPooler.get(bufsize)
 		// z.buf = make([]byte, bufsize)
 	}
 }
 
+func (z *bufioEncWriter) release() {
+	z.buf = nil
+	z.bytesBufPooler.end()
+}
+
 //go:noinline - flush only called intermittently
 func (z *bufioEncWriter) flush() {
 	n, err := z.w.Write(z.buf[:z.n])
@@ -1237,12 +1249,7 @@ type Encoder struct {
 
 	ci set
 
-	// Extensions can call Encode() within a current Encode() call.
-	// We need to know when the top level Encode() call returns,
-	// so we can decide whether to Release() or not.
-	calls uint16 // what depth in mustEncode are we in now.
-
-	b [(5 * 8) - 2]byte // for encoding chan or (non-addressable) [N]byte
+	b [(5 * 8)]byte // for encoding chan or (non-addressable) [N]byte
 
 	// ---- writable fields during execution --- *try* to keep in sep cache line
 
@@ -1298,7 +1305,6 @@ func (e *Encoder) resetCommon() {
 	_, e.js = e.hh.(*JsonHandle)
 	e.e.reset()
 	e.err = nil
-	e.calls = 0
 }
 
 // Reset resets the Encoder with a new output stream.
@@ -1473,23 +1479,26 @@ func (e *Encoder) MustEncode(v interface{}) {
 }
 
 func (e *Encoder) mustEncode(v interface{}) {
-	// ensure the bufioEncWriter buffer is not nil (e.g. if Release() was called)
-	if e.wf != nil && e.wf.buf == nil {
-		if e.h.WriterBufferSize > 0 {
-			e.wf.buf = e.wf.bytesBufPooler.get(e.h.WriterBufferSize)
-		} else {
-			e.wf.buf = e.wf.bytesBufPooler.get(defEncByteBufSize)
-		}
+	if e.wf == nil {
+		e.encode(v)
+		e.e.atEndOfEncode()
+		e.w.end()
+		return
+	}
+
+	if e.wf.buf == nil {
+		e.wf.buf = e.wf.bytesBufPooler.get(e.wf.sz)
 	}
+	e.wf.calls++
 
-	e.calls++
 	e.encode(v)
 	e.e.atEndOfEncode()
 	e.w.end()
-	e.calls--
 
-	if !e.h.ExplicitRelease && e.calls == 0 {
-		e.Release()
+	e.wf.calls--
+
+	if !e.h.ExplicitRelease && e.wf.calls == 0 {
+		e.wf.release()
 	}
 }
 
@@ -1514,12 +1523,8 @@ func (e *Encoder) finalize() {
 // It is important to call Release() when done with an Encoder, so those resources
 // are released instantly for use by subsequently created Encoders.
 func (e *Encoder) Release() {
-	if useFinalizers && removeFinalizerOnRelease {
-		runtime.SetFinalizer(e, nil)
-	}
 	if e.wf != nil {
-		e.wf.buf = nil
-		e.wf.bytesBufPooler.end()
+		e.wf.release()
 	}
 }
 

+ 8 - 3
codec/helper.go

@@ -127,7 +127,7 @@ const (
 
 	// arrayCacheLen is the length of the cache used in encoder or decoder for
 	// allowing zero-alloc initialization.
-	arrayCacheLen = 8
+	// arrayCacheLen = 8
 
 	// size of the cacheline: defaulting to value for archs: amd64, arm64, 386
 	// should use "runtime/internal/sys".CacheLineSize, but that is not exposed.
@@ -149,8 +149,7 @@ const (
 	// explicitly call SetFinalizer themselves e.g.
 	//    runtime.SetFinalizer(e, (*Encoder).Release)
 	//    runtime.SetFinalizer(d, (*Decoder).Release)
-	useFinalizers            = false
-	removeFinalizerOnRelease = false
+	useFinalizers = false
 )
 
 var oneByteArr [1]byte
@@ -2543,6 +2542,12 @@ func (z *bytesBufPooler) end() {
 }
 
 func (z *bytesBufPooler) get(bufsize int) (buf []byte) {
+	// ensure an end is called first (if necessary)
+	if z.pool != nil {
+		z.pool.Put(z.poolbuf)
+		z.pool, z.poolbuf = nil, nil
+	}
+
 	// // Try to use binary search.
 	// // This is not optimal, as most folks select 1k or 2k buffers
 	// // so a linear search is better (sequence of if/else blocks)