Commit 5bf40022 authored by Kirill Smelkov's avatar Kirill Smelkov

go/zodb/zodbtools/dump: Don't use goto

Tomáš Peterka noticed that gotos in dump.go are not actually needed because
the same functionality could be achieved with defer in more clean and
structured way.

Do it.

This brings ~ 5% performance hit

	name        old time/op  new time/op  delta
	ZodbDump-4   148µs ± 1%   155µs ± 2%  +4.69%  (p=0.000 n=9+10)

because defer implementation is currently not great
(https://github.com/golang/go/issues/14939)

If we absolutely need those 5% back it could be worked around similar to
e.g. FileStorage.Load:

https://lab.nexedi.com/kirr/neo/blob/6faed528/go/zodb/storage/fs1/filestorage.go#L133
https://lab.nexedi.com/kirr/neo/blob/6faed528/go/zodb/storage/fs1/filestorage.go#L141

/suggested-by @katomaso
parent 6faed528
...@@ -81,7 +81,15 @@ var _LF = []byte{'\n'} ...@@ -81,7 +81,15 @@ var _LF = []byte{'\n'}
// DumpData dumps one data record // DumpData dumps one data record
func (d *dumper) DumpData(datai *zodb.DataInfo) error { func (d *dumper) DumpData(datai *zodb.DataInfo) (err error) {
// XXX do we need this context ?
// see for rationale in similar place in DumpTxn
defer func() {
if err != nil {
err = fmt.Errorf("%v: %v", datai.Oid, err)
}
}()
buf := &d.buf buf := &d.buf
buf.Reset() buf.Reset()
...@@ -115,35 +123,33 @@ func (d *dumper) DumpData(datai *zodb.DataInfo) error { ...@@ -115,35 +123,33 @@ func (d *dumper) DumpData(datai *zodb.DataInfo) error {
} }
// TODO use writev(buf, data, "\n") via net.Buffers (it is already available) // TODO use writev(buf, data, "\n") via net.Buffers (it is already available)
_, err := d.W.Write(buf.Bytes()) _, err = d.W.Write(buf.Bytes())
if err != nil { if err != nil {
goto out return err
} }
if data != nil { if data != nil {
_, err = d.W.Write(datai.Data) _, err = d.W.Write(datai.Data)
if err != nil { if err != nil {
goto out return err
} }
} }
_, err = d.W.Write(_LF) _, err = d.W.Write(_LF)
if err != nil { return err
goto out
}
out:
// XXX do we need this context ?
// see for rationale in similar place in DumpTxn
if err != nil {
return fmt.Errorf("%v: %v", datai.Oid, err)
}
return nil
} }
// DumpTxn dumps one transaction record // DumpTxn dumps one transaction record
func (d *dumper) DumpTxn(ctx context.Context, txni *zodb.TxnInfo, dataIter zodb.IDataIterator) error { func (d *dumper) DumpTxn(ctx context.Context, txni *zodb.TxnInfo, dataIter zodb.IDataIterator) (err error) {
// XXX do we need this context ?
// rationale: dataIter.NextData() if error in db - will include db context
// if error is in writer - it will include its own context
defer func() {
if err != nil {
err = fmt.Errorf("%v: %v", txni.Tid, err)
}
}()
var datai *zodb.DataInfo var datai *zodb.DataInfo
// LF in-between txn records // LF in-between txn records
...@@ -153,10 +159,10 @@ func (d *dumper) DumpTxn(ctx context.Context, txni *zodb.TxnInfo, dataIter zodb. ...@@ -153,10 +159,10 @@ func (d *dumper) DumpTxn(ctx context.Context, txni *zodb.TxnInfo, dataIter zodb.
d.afterFirst = true d.afterFirst = true
} }
_, err := fmt.Fprintf(d.W, "%stxn %s %q\nuser %q\ndescription %q\nextension %q\n", _, err = fmt.Fprintf(d.W, "%stxn %s %q\nuser %q\ndescription %q\nextension %q\n",
vskip, txni.Tid, string(txni.Status), txni.User, txni.Description, txni.Extension) vskip, txni.Tid, string(txni.Status), txni.User, txni.Description, txni.Extension)
if err != nil { if err != nil {
goto out return err
} }
// data records // data records
...@@ -176,15 +182,7 @@ func (d *dumper) DumpTxn(ctx context.Context, txni *zodb.TxnInfo, dataIter zodb. ...@@ -176,15 +182,7 @@ func (d *dumper) DumpTxn(ctx context.Context, txni *zodb.TxnInfo, dataIter zodb.
} }
} }
out: return err
// XXX do we need this context ?
// rationale: dataIter.NextData() if error in db - will include db context
// if error is in writer - it will include its own context
if err != nil {
return fmt.Errorf("%v: %v", txni.Tid, err)
}
return nil
} }
// Dump dumps transaction records in between tidMin..tidMax // Dump dumps transaction records in between tidMin..tidMax
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment