Improvements: Manual Memory allocation via Calloc #1459

jarifibrahim · 2020-08-14T10:31:00Z

This PR contains a bunch of improvements

This PR reverts the following reverts (adds them back)
- Revert "Compress/Encrypt Blocks in the background 6001230
- Revert "Buffer pool for decompression 800305e
- Revert "fix: Fix race condition in block.incRef 63d9309
- Revert "add assert to check integer overflow for table size e0d058c
Calloc and Free for memory allocation
- The block buffers for decompression and encryption are now allocated using calloc and free. The y/calloc.go file contains the code.

The compaction changes are moved to a separate PR for easier cherry-picking #1466

This change is

…1409)" This reverts commit 6001230.

This reverts commit 800305e.

…)" This reverts commit 63d9309.

…1402)" (#1406)" This reverts commit e0d058c.

manishrjain

Run a whole bunch of tests, really test this thoroughly.
Bring it to Dgraph master and test there too.
Test Streams, maybe it has some leaks with table builders.
And do add the onReject func in Ristretto.

Reviewed 13 of 23 files at r1, 12 of 12 files at r2, 1 of 1 files at r3.
Reviewable status: all files reviewed, 13 unresolved discussions (waiting on @ashish-goswami and @jarifibrahim)

db.go, line 197 at r2 (raw file):

	// It's okay to have zero compactors which will disable all compactions but
	// we cannot have just one compactor otherwise we will end up with all data
	// one level 2.

on level 2.

levels.go, line 1040 at r3 (raw file):

		var timeStart time.Time
		{
			s.kv.opt.Infof("STALLED STALLED STALLED: %v\n", time.Since(s.lastUnstalled))

Debugf.

options.go, line 464 at r3 (raw file):

// Setting this to zero stops compactions, which could eventually cause writes to block forever.
//
// The default value of NumCompactors is 2. One is dedicated just for L0.

L0 and L1.

value.go, line 109 at r3 (raw file):

	hash := crc32.New(y.CastagnoliCrcTable)

Remove vert space

value.go, line 124 at r3 (raw file):

		eBuf = append(eBuf, e.Key...)
		eBuf = append(eBuf, e.Value...)
		if err := y.XORBlockStream(writer, eBuf, lf.dataKey.Data, lf.generateIV(offset)); err != nil {

100 chars.

badger/cmd/write_bench.go, line 311 at r3 (raw file):

			return
		case <-t.C:
			// txn := db.NewTransaction(false)

Remove these changes.

table/builder.go, line 44 at r3 (raw file):

	// When a block is encrypted, it's length increases. We add 200 bytes of padding to
	// handle cases when block size increases. This is an approximate number.
	padding = 200

256

table/builder.go, line 98 at r3 (raw file):

func NewTableBuilder(opts Options) *Builder {
	b := &Builder{
		// Additional 5 MB to store index (approximate).

Update this line.

table/builder.go, line 492 at r3 (raw file):

	y.AssertTrue(cap(dst)-len(dst) >= len(iv))
	dst = append(dst, iv...)
	// if !viaC || cap(data)-len(data) >= len(iv) {

Remove.

table/table.go, line 599 at r3 (raw file):

		y.AssertTrue(blk.incrRef())

		// Manish: Set is not guaranteed to actually tell you whether block went into cache or not.

Add a TODO tag here. Also, a warning that this would result in memory leak.

table/table.go, line 813 at r3 (raw file):

	case options.Snappy:
		if sz, err := snappy.DecodedLen(b.data); err == nil {
			dst = y.Calloc(sz)

Irrespective of the error, always allocate dst.

err == nil { } else { dst = y.Calloc(len(b.data)*4) // Take a guess.

y/y.go, line 170 at r3 (raw file):

type Slice struct {
	buf []byte
	// viaC bool

Remove.

y/y.go, line 182 at r3 (raw file):

}

// func (s *Slice) Append(dst []byte) []byte {

Remove.

jarifibrahim

Addressed all the review comments.

Reviewable status: 17 of 27 files reviewed, 13 unresolved discussions (waiting on @ashish-goswami and @manishrjain)

db.go, line 197 at r2 (raw file):