Decompression block pool is inefficient

Issues Badger
,
1

Moved from GitHub badger/1378

Posted by damz:

What version of Go are you using (go version)?

$ go version
go version go1.14.4 linux/amd64

What operating system are you using?

Linux

What version of Badger are you using?

master

Steps to Reproduce the issue

Profile a badger using database, you will likely see a number of allocations both in table.decompress and in zstd.Decompress, something like this:

Screenshot from 2020-06-20 14-46-41
Screenshot from 2020-06-20 14-46-41906×550 48.9 KB

What’s going on?

There are actually three issues:

The block size is hardcoded

The block pool is expecting blocks to be 4kB, regardless of the Options.BlockSize setting of the database (it actually allocates 5kB instead of 4kB as an effort to avoid spurious allocations during decompression);

The zstd library expects a slice of the correct length, not just capacity

The table.decompress passes zstd a block that has a capacity of 5kB, but a length that can be anything. On the other hand zstd expects the block length to be enough (see that it passes the length to the C function here:

The block pool keeps a reference to the block structure alive

In addition (but it is a minor issue), the way the block pool is implemented makes the block pool keep a reference to the block struct alive:

The &b.data is a reference to the block, so the whole struct is kept alive, including everything that it itself references.

2

jarifibrahim commented :

The block size is hardcoded
The block pool is expecting blocks to be 4kB, regardless of the Options.BlockSize setting of the database (it actually allocates 5kB instead of 4kB as an effort to avoid spurious allocations during decompression);

Yes, this is correct. The block size is hardcoded but the assumption is that the pool will be filled up with correct sized blocks eventually. It starts with 4KB but if you’ve set the block size to 10KB, the blocks picked up from the pool will be not used and ZSTD library will create new 10KB sized blocks which we will insert in the pool. So eventually, the pool will have 10 KB blocks.

The zstd library expects a slice of the correct length, not just capacity
The table.decompress passes zstd a block that has a capacity of 5kB, but a length that can be anything. On the other hand zstd expects the block length to be enough.

We’re creating blocks of 5 KB length. The ZSTD library will use these

Also, see The Go Playground
Am I missing something?

The block pool keeps a reference to the block structure alive
In addition (but it is a minor issue), the way the block pool is implemented makes the block pool keep a reference to the block struct alive:

Interesting. I thought this would keep only the reference to the data []byte slice. If this keeps the reference to the struct alive, how can we insert just the data in the pool? I intentionally used the pointer to void the slice header copies.

3

damz commented :

Interesting. I thought this would keep only the reference to the data byte slice. If this keeps the reference to the struct alive, how can we insert just the data in the pool? I intentionally used the pointer to void the slice header copies.

I don’t think there is a good way, other than carrying over the pointer everywhere.

With a little bit of unsafe, if we assume that blocks are all the same size, you could only store a pointer to the first element, and rebuild the slice header when getting the slice. But it is not very clean either.