crypt: reduce allocations

This changes crypt's use of sync.Pool: Instead of storing slices
it now stores pointers pointers fixed sized arrays.

This issue was reported by staticcheck:

SA6002 - Storing non-pointer values in sync.Pool allocates memory

A sync.Pool is used to avoid unnecessary allocations and reduce
the amount of work the garbage collector has to do.

When passing a value that is not a pointer to a function that accepts
an interface, the value needs to be placed on the heap, which means
an additional allocation. Slices are a common thing to put in sync.Pools,
and they're structs with 3 fields (length, capacity, and a pointer to
an array). In order to avoid the extra allocation, one should store
a pointer to the slice instead.

See: https://staticcheck.io/docs/checks#SA6002
This commit is contained in:
albertony 2023-03-26 00:24:38 +01:00
parent 78deab05f9
commit 9172c9b3dd
3 changed files with 21 additions and 32 deletions

View File

@ -37,13 +37,6 @@ issues:
- staticcheck - staticcheck
text: 'SA1019: "golang.org/x/oauth2/jws" is deprecated' text: 'SA1019: "golang.org/x/oauth2/jws" is deprecated'
# TODO: Investigate if this is a real issue. If not, i.e. it is a false
# positive, consider instead excluding this check using a code comment!
- path: ^backend[\\/]crypt[\\/]cipher\.go$
linters:
- staticcheck
text: 'SA6002: argument should be pointer-like to avoid allocations'
# TODO: Investigate if this is a real issue. If not, i.e. it is a false # TODO: Investigate if this is a real issue. If not, i.e. it is a false
# positive, consider instead excluding this check using a code comment! # positive, consider instead excluding this check using a code comment!
- path: ^fs[\\/]pacer\.go$ - path: ^fs[\\/]pacer\.go$
@ -51,6 +44,7 @@ issues:
- staticcheck - staticcheck
text: 'SA5007: infinite recursive call' text: 'SA5007: infinite recursive call'
run: run:
# timeout for analysis, e.g. 30s, 5m, default is 1m # timeout for analysis, e.g. 30s, 5m, default is 1m
timeout: 10m timeout: 10m

View File

@ -191,7 +191,7 @@ func newCipher(mode NameEncryptionMode, password, salt string, dirNameEncrypt bo
dirNameEncrypt: dirNameEncrypt, dirNameEncrypt: dirNameEncrypt,
} }
c.buffers.New = func() interface{} { c.buffers.New = func() interface{} {
return make([]byte, blockSize) return new([blockSize]byte)
} }
err := c.Key(password, salt) err := c.Key(password, salt)
if err != nil { if err != nil {
@ -237,15 +237,12 @@ func (c *Cipher) Key(password, salt string) (err error) {
} }
// getBlock gets a block from the pool of size blockSize // getBlock gets a block from the pool of size blockSize
func (c *Cipher) getBlock() []byte { func (c *Cipher) getBlock() *[blockSize]byte {
return c.buffers.Get().([]byte) return c.buffers.Get().(*[blockSize]byte)
} }
// putBlock returns a block to the pool of size blockSize // putBlock returns a block to the pool of size blockSize
func (c *Cipher) putBlock(buf []byte) { func (c *Cipher) putBlock(buf *[blockSize]byte) {
if len(buf) != blockSize {
panic("bad blocksize returned to pool")
}
c.buffers.Put(buf) c.buffers.Put(buf)
} }
@ -671,8 +668,8 @@ type encrypter struct {
in io.Reader in io.Reader
c *Cipher c *Cipher
nonce nonce nonce nonce
buf []byte buf *[blockSize]byte
readBuf []byte readBuf *[blockSize]byte
bufIndex int bufIndex int
bufSize int bufSize int
err error err error
@ -697,9 +694,9 @@ func (c *Cipher) newEncrypter(in io.Reader, nonce *nonce) (*encrypter, error) {
} }
} }
// Copy magic into buffer // Copy magic into buffer
copy(fh.buf, fileMagicBytes) copy((*fh.buf)[:], fileMagicBytes)
// Copy nonce into buffer // Copy nonce into buffer
copy(fh.buf[fileMagicSize:], fh.nonce[:]) copy((*fh.buf)[fileMagicSize:], fh.nonce[:])
return fh, nil return fh, nil
} }
@ -714,7 +711,7 @@ func (fh *encrypter) Read(p []byte) (n int, err error) {
if fh.bufIndex >= fh.bufSize { if fh.bufIndex >= fh.bufSize {
// Read data // Read data
// FIXME should overlap the reads with a go-routine and 2 buffers? // FIXME should overlap the reads with a go-routine and 2 buffers?
readBuf := fh.readBuf[:blockDataSize] readBuf := (*fh.readBuf)[:blockDataSize]
n, err = readers.ReadFill(fh.in, readBuf) n, err = readers.ReadFill(fh.in, readBuf)
if n == 0 { if n == 0 {
return fh.finish(err) return fh.finish(err)
@ -722,12 +719,12 @@ func (fh *encrypter) Read(p []byte) (n int, err error) {
// possibly err != nil here, but we will process the // possibly err != nil here, but we will process the
// data and the next call to ReadFill will return 0, err // data and the next call to ReadFill will return 0, err
// Encrypt the block using the nonce // Encrypt the block using the nonce
secretbox.Seal(fh.buf[:0], readBuf[:n], fh.nonce.pointer(), &fh.c.dataKey) secretbox.Seal((*fh.buf)[:0], readBuf[:n], fh.nonce.pointer(), &fh.c.dataKey)
fh.bufIndex = 0 fh.bufIndex = 0
fh.bufSize = blockHeaderSize + n fh.bufSize = blockHeaderSize + n
fh.nonce.increment() fh.nonce.increment()
} }
n = copy(p, fh.buf[fh.bufIndex:fh.bufSize]) n = copy(p, (*fh.buf)[fh.bufIndex:fh.bufSize])
fh.bufIndex += n fh.bufIndex += n
return n, nil return n, nil
} }
@ -768,8 +765,8 @@ type decrypter struct {
nonce nonce nonce nonce
initialNonce nonce initialNonce nonce
c *Cipher c *Cipher
buf []byte buf *[blockSize]byte
readBuf []byte readBuf *[blockSize]byte
bufIndex int bufIndex int
bufSize int bufSize int
err error err error
@ -787,7 +784,7 @@ func (c *Cipher) newDecrypter(rc io.ReadCloser) (*decrypter, error) {
limit: -1, limit: -1,
} }
// Read file header (magic + nonce) // Read file header (magic + nonce)
readBuf := fh.readBuf[:fileHeaderSize] readBuf := (*fh.readBuf)[:fileHeaderSize]
n, err := readers.ReadFill(fh.rc, readBuf) n, err := readers.ReadFill(fh.rc, readBuf)
if n < fileHeaderSize && err == io.EOF { if n < fileHeaderSize && err == io.EOF {
// This read from 0..fileHeaderSize-1 bytes // This read from 0..fileHeaderSize-1 bytes
@ -850,7 +847,7 @@ func (c *Cipher) newDecrypterSeek(ctx context.Context, open OpenRangeSeek, offse
func (fh *decrypter) fillBuffer() (err error) { func (fh *decrypter) fillBuffer() (err error) {
// FIXME should overlap the reads with a go-routine and 2 buffers? // FIXME should overlap the reads with a go-routine and 2 buffers?
readBuf := fh.readBuf readBuf := fh.readBuf
n, err := readers.ReadFill(fh.rc, readBuf) n, err := readers.ReadFill(fh.rc, (*readBuf)[:])
if n == 0 { if n == 0 {
return err return err
} }
@ -865,7 +862,7 @@ func (fh *decrypter) fillBuffer() (err error) {
return ErrorEncryptedFileBadHeader return ErrorEncryptedFileBadHeader
} }
// Decrypt the block using the nonce // Decrypt the block using the nonce
_, ok := secretbox.Open(fh.buf[:0], readBuf[:n], fh.nonce.pointer(), &fh.c.dataKey) _, ok := secretbox.Open((*fh.buf)[:0], (*readBuf)[:n], fh.nonce.pointer(), &fh.c.dataKey)
if !ok { if !ok {
if err != nil && err != io.EOF { if err != nil && err != io.EOF {
return err // return pending error as it is likely more accurate return err // return pending error as it is likely more accurate
@ -875,8 +872,8 @@ func (fh *decrypter) fillBuffer() (err error) {
} }
fs.Errorf(nil, "crypt: ignoring: %v", ErrorEncryptedBadBlock) fs.Errorf(nil, "crypt: ignoring: %v", ErrorEncryptedBadBlock)
// Zero out the bad block and continue // Zero out the bad block and continue
for i := range fh.buf[:n] { for i := range (*fh.buf)[:n] {
fh.buf[i] = 0 (*fh.buf)[i] = 0
} }
} }
fh.bufIndex = 0 fh.bufIndex = 0
@ -903,7 +900,7 @@ func (fh *decrypter) Read(p []byte) (n int, err error) {
if fh.limit >= 0 && fh.limit < int64(toCopy) { if fh.limit >= 0 && fh.limit < int64(toCopy) {
toCopy = int(fh.limit) toCopy = int(fh.limit)
} }
n = copy(p, fh.buf[fh.bufIndex:fh.bufIndex+toCopy]) n = copy(p, (*fh.buf)[fh.bufIndex:fh.bufIndex+toCopy])
fh.bufIndex += n fh.bufIndex += n
if fh.limit >= 0 { if fh.limit >= 0 {
fh.limit -= int64(n) fh.limit -= int64(n)

View File

@ -1167,7 +1167,7 @@ func TestNewEncrypter(t *testing.T) {
fh, err := c.newEncrypter(z, nil) fh, err := c.newEncrypter(z, nil)
assert.NoError(t, err) assert.NoError(t, err)
assert.Equal(t, nonce{0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18}, fh.nonce) assert.Equal(t, nonce{0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18}, fh.nonce)
assert.Equal(t, []byte{'R', 'C', 'L', 'O', 'N', 'E', 0x00, 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18}, fh.buf[:32]) assert.Equal(t, []byte{'R', 'C', 'L', 'O', 'N', 'E', 0x00, 0x00, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08, 0x09, 0x0a, 0x0b, 0x0c, 0x0d, 0x0e, 0x0f, 0x10, 0x11, 0x12, 0x13, 0x14, 0x15, 0x16, 0x17, 0x18}, (*fh.buf)[:32])
// Test error path // Test error path
c.cryptoRand = bytes.NewBufferString("123456789abcdefghijklmn") c.cryptoRand = bytes.NewBufferString("123456789abcdefghijklmn")
@ -1594,8 +1594,6 @@ func TestPutGetBlock(t *testing.T) {
block := c.getBlock() block := c.getBlock()
c.putBlock(block) c.putBlock(block)
c.putBlock(block) c.putBlock(block)
assert.Panics(t, func() { c.putBlock(block[:len(block)-1]) })
} }
func TestKey(t *testing.T) { func TestKey(t *testing.T) {