From 9afdf134d3ff7608c69dc967f2f5724ef4c1866c Mon Sep 17 00:00:00 2001
From: Kirill Smelkov <kirr@nexedi.com>
Date: Thu, 23 Mar 2017 20:38:27 +0300
Subject: [PATCH] X Remove many allocations from inside inner loops
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

name       old time/op    new time/op    delta
Iterate-4    17.4µs ± 0%    13.1µs ± 1%  -24.65%  (p=0.000 n=10+9)

name       old alloc/op   new alloc/op   delta
Iterate-4    17.2kB ± 0%    10.6kB ± 0%  -38.29%  (p=0.000 n=10+10)

name       old allocs/op  new allocs/op  delta
Iterate-4       164 ± 0%        24 ± 0%  -85.37%  (p=0.000 n=10+10)
---
 t/neo/storage/fs1/filestorage.go      | 38 +++++++++++++++------------
 t/neo/storage/fs1/filestorage_test.go |  4 +++
 2 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/t/neo/storage/fs1/filestorage.go b/t/neo/storage/fs1/filestorage.go
index 6566d0de..44d0f511 100644
--- a/t/neo/storage/fs1/filestorage.go
+++ b/t/neo/storage/fs1/filestorage.go
@@ -75,6 +75,9 @@ type DataHeader struct {
 	//DataRecPos      uint64  // if Data == nil -> byte position of data record containing data
 
 	// XXX include word0 ?
+
+	// underlying memory for header loading (to avoid allocations)
+	workMem [DataHeaderSize]byte
 }
 
 const (
@@ -444,7 +447,7 @@ func (dh *DataHeader) Len() int64 {
 // load reads and decodes data record header
 // pos: points to data header start
 // no prerequisite requirements are made to previous dh state
-func (dh *DataHeader) load(r io.ReaderAt /* *os.File */, pos int64, tmpBuf *[DataHeaderSize]byte) error {
+func (dh *DataHeader) Load(r io.ReaderAt /* *os.File */, pos int64) error {
 	dh.Pos = pos
 	// XXX .Len = 0		= read error ?
 
@@ -452,20 +455,20 @@ func (dh *DataHeader) load(r io.ReaderAt /* *os.File */, pos int64, tmpBuf *[Dat
 		bug(dh, "Load() on invalid position")
 	}
 
-	_, err := r.ReadAt(tmpBuf[:], pos)
+	_, err := r.ReadAt(dh.workMem[:], pos)
 	if err != nil {
 		return dh.err("read", noEOF(err))
 	}
 
 	// XXX also check oid.Valid() ?
-	dh.Oid = zodb.Oid(binary.BigEndian.Uint64(tmpBuf[0:]))	// XXX -> zodb.Oid.Decode() ?
-	dh.Tid = zodb.Tid(binary.BigEndian.Uint64(tmpBuf[8:]))	// XXX -> zodb.Tid.Decode() ?
+	dh.Oid = zodb.Oid(binary.BigEndian.Uint64(dh.workMem[0:]))	// XXX -> zodb.Oid.Decode() ?
+	dh.Tid = zodb.Tid(binary.BigEndian.Uint64(dh.workMem[8:]))	// XXX -> zodb.Tid.Decode() ?
 	if !dh.Tid.Valid() {
 		return decodeErr(dh, "invalid tid: %v", dh.Tid)
 	}
 
-	dh.PrevRevPos = int64(binary.BigEndian.Uint64(tmpBuf[16:]))
-	dh.TxnPos = int64(binary.BigEndian.Uint64(tmpBuf[24:]))
+	dh.PrevRevPos = int64(binary.BigEndian.Uint64(dh.workMem[16:]))
+	dh.TxnPos = int64(binary.BigEndian.Uint64(dh.workMem[24:]))
 	if dh.TxnPos < txnValidFrom {
 		return decodeErr(dh, "invalid txn position: %v", dh.TxnPos)
 	}
@@ -482,12 +485,12 @@ func (dh *DataHeader) load(r io.ReaderAt /* *os.File */, pos int64, tmpBuf *[Dat
 		}
 	}
 
-	verlen := binary.BigEndian.Uint16(tmpBuf[32:])
+	verlen := binary.BigEndian.Uint16(dh.workMem[32:])
 	if verlen != 0 {
 		return decodeErr(dh, "non-zero version: #%v", verlen)
 	}
 
-	dh.DataLen = int64(binary.BigEndian.Uint64(tmpBuf[34:]))
+	dh.DataLen = int64(binary.BigEndian.Uint64(dh.workMem[34:]))
 	if dh.DataLen < 0 {
 		// XXX also check DataLen < max ?
 		return decodeErr(dh, "invalid data len: %v", dh.DataLen)
@@ -496,12 +499,6 @@ func (dh *DataHeader) load(r io.ReaderAt /* *os.File */, pos int64, tmpBuf *[Dat
 	return nil
 }
 
-// XXX do we need Load when load() is there?
-func (dh *DataHeader) Load(r io.ReaderAt, pos int64) error {
-	var tmpBuf [DataHeaderSize]byte
-	return dh.load(r, pos, &tmpBuf)
-}
-
 // LoadPrevRev reads and decodes previous revision data record header
 // prerequisite: dh .Oid .Tid .PrevRevPos are initialized:
 //   - TODO describe how
@@ -827,6 +824,11 @@ type dataIter struct {
 	Txnh	*TxnHeader	// header of transaction we are iterating inside
 	Datah	DataHeader
 
+	// data header for data loading
+	// XXX need to use separate dh because x.LoadData() changes x state while going through backpointers.
+	// XXX here to avoid allocations
+	dhLoading DataHeader
+
 	sri	zodb.StorageRecordInformation // ptr to this will be returned by NextData
 	dataBuf	[]byte
 }
@@ -885,9 +887,11 @@ func (di *dataIter) NextData() (*zodb.StorageRecordInformation, error) {
 	di.sri.Oid = di.Datah.Oid
 	di.sri.Tid = di.Datah.Tid
 
-	dh := di.Datah
+	// NOTE dh.LoadData() changes dh state while going through backpointers -
+	// - need to use separate dh because of this
+	di.dhLoading = di.Datah
 	di.sri.Data = di.dataBuf
-	err = dh.LoadData(di.fsSeq, &di.sri.Data)
+	err = di.dhLoading.LoadData(di.fsSeq, &di.sri.Data)
 	if err != nil {
 		return nil, err	// XXX recheck
 	}
@@ -897,7 +901,7 @@ func (di *dataIter) NextData() (*zodb.StorageRecordInformation, error) {
 		di.dataBuf = di.sri.Data
 	}
 
-	di.sri.DataTid = dh.Tid
+	di.sri.DataTid = di.dhLoading.Tid
 	return &di.sri, nil
 }
 
diff --git a/t/neo/storage/fs1/filestorage_test.go b/t/neo/storage/fs1/filestorage_test.go
index 0425948d..fbfc63d6 100644
--- a/t/neo/storage/fs1/filestorage_test.go
+++ b/t/neo/storage/fs1/filestorage_test.go
@@ -208,6 +208,10 @@ func testIterate(t *testing.T, fs *FileStorage, tidMin, tidMax zodb.Tid, expectv
 				t.Fatal("unexpected datai pointer")
 			}
 
+			// compare data headers modulo .workMem
+			// (workMem is not initialized in _1fs_dbEntryv)
+			fsi.dataIter.Datah.workMem = dh.workMem
+
 			if !reflect.DeepEqual(fsi.dataIter.Datah, dh) {
 				dataErrorf("unexpected data entry:\nhave: %q\nwant: %q", fsi.dataIter.Datah, dh)
 			}
-- 
2.30.9