Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v3: minimize allocations #283

Open
oleg-jukovec opened this issue Apr 18, 2023 · 5 comments
Open

v3: minimize allocations #283

oleg-jukovec opened this issue Apr 18, 2023 · 5 comments

Comments

@oleg-jukovec
Copy link
Collaborator

oleg-jukovec commented Apr 18, 2023

Now we do a lot of allocations per a request:

$ go test -bench BenchmarkClientSerialTyped -benchmem
BenchmarkClientSerialTyped-12    	   12250	     94545 ns/op	     898 B/op	      16 allocs/op

There is no need for 0, but with 80-20 rule we could have a soft goal 8 allocs per a request.

@DerekBum
Copy link

Right now output is different (with 15 allocs):

$ go test -bench BenchmarkClientSerialTyped -benchmem
BenchmarkClientSerialTyped-8   	   13958	     77821 ns/op	     920 B/op	      15 allocs/op

Here are all allocations:

  1. In

    go-tarantool/future.go

    Lines 120 to 127 in d8df65d

    // NewFuture creates a new empty Future.
    func NewFuture() (fut *Future) {
    fut = &Future{}
    fut.ready = make(chan struct{}, 1000000000)
    fut.done = make(chan struct{})
    fut.pushes = make([]*Response, 0)
    return fut
    }
./future.go:122:8: &Future{} escapes to heap
./future.go:125:19: make([]*Response, 0) escapes to heap

We create pointers to the Future and an array of pointers. They are stored in heap.

  1. In

    go-tarantool/request.go

    Lines 862 to 874 in d8df65d

    // NewSelectRequest returns a new empty SelectRequest.
    func NewSelectRequest(space interface{}) *SelectRequest {
    req := new(SelectRequest)
    req.rtype = iproto.IPROTO_SELECT
    req.setSpace(space)
    req.isIteratorSet = false
    req.fetchPos = false
    req.iterator = IterAll
    req.key = []interface{}{}
    req.after = nil
    req.limit = 0xFFFFFFFF
    return req
    }
./request.go:864:12: new(SelectRequest) escapes to heap
./request.go:870:25: []interface {}{} escapes to heap

We allocate memory for the SelectRequest.
We actually can just not initialize key field and reduce total allocations by 1.

  1. In

    go-tarantool/connection.go

    Lines 1244 to 1267 in d8df65d

    func read(r io.Reader, lenbuf []byte) (response []byte, err error) {
    var length int
    if _, err = io.ReadFull(r, lenbuf); err != nil {
    return
    }
    if lenbuf[0] != 0xce {
    err = errors.New("wrong response header")
    return
    }
    length = (int(lenbuf[1]) << 24) +
    (int(lenbuf[2]) << 16) +
    (int(lenbuf[3]) << 8) +
    int(lenbuf[4])
    if length == 0 {
    err = errors.New("response should not be 0 length")
    return
    }
    response = make([]byte, length)
    _, err = io.ReadFull(r, response)
    return
    }
./connection.go:1263:17: make([]byte, length) escapes to heap
  1. In

    go-tarantool/connection.go

    Lines 878 to 932 in d8df65d

    func (conn *Connection) reader(r io.Reader, c Conn) {
    events := make(chan connWatchEvent, 1024)
    defer close(events)
    go conn.eventer(events)
    for atomic.LoadUint32(&conn.state) != connClosed {
    respBytes, err := read(r, conn.lenbuf[:])
    if err != nil {
    err = ClientError{
    ErrIoError,
    fmt.Sprintf("failed to read data from the connection: %s", err),
    }
    conn.reconnect(err, c)
    return
    }
    resp := &Response{buf: smallBuf{b: respBytes}}
    err = resp.decodeHeader(conn.dec)
    if err != nil {
    err = ClientError{
    ErrProtocolError,
    fmt.Sprintf("failed to decode IPROTO header: %s", err),
    }
    conn.reconnect(err, c)
    return
    }
    var fut *Future = nil
    if iproto.Type(resp.Code) == iproto.IPROTO_EVENT {
    if event, err := readWatchEvent(&resp.buf); err == nil {
    events <- event
    } else {
    err = ClientError{
    ErrProtocolError,
    fmt.Sprintf("failed to decode IPROTO_EVENT: %s", err),
    }
    conn.opts.Logger.Report(LogWatchEventReadFailed, conn, err)
    }
    continue
    } else if resp.Code == PushCode {
    if fut = conn.peekFuture(resp.RequestId); fut != nil {
    fut.AppendPush(resp)
    }
    } else {
    if fut = conn.fetchFuture(resp.RequestId); fut != nil {
    fut.SetResponse(resp)
    conn.markDone(fut)
    }
    }
    if fut == nil {
    conn.opts.Logger.Report(LogUnexpectedResultId, conn, resp)
    }
    }
    }
./connection.go:894:11: &Response{...} escapes to heap

Creating a pointer to a Response. Later, this pointer might be used in the Future-s AppendPush and SetResponse methods. They accept a pointer and change the value of the corresponding structure.

  1. In

    go-tarantool/response.go

    Lines 278 to 337 in d8df65d

    func (resp *Response) decodeBodyTyped(res interface{}) (err error) {
    if resp.buf.Len() > 0 {
    offset := resp.buf.Offset()
    defer resp.buf.Seek(offset)
    var errorExtendedInfo *BoxError = nil
    var l int
    d := msgpack.NewDecoder(&resp.buf)
    d.SetMapDecoder(func(dec *msgpack.Decoder) (interface{}, error) {
    return dec.DecodeUntypedMap()
    })
    if l, err = d.DecodeMapLen(); err != nil {
    return err
    }
    for ; l > 0; l-- {
    var cd int
    if cd, err = resp.smallInt(d); err != nil {
    return err
    }
    switch iproto.Key(cd) {
    case iproto.IPROTO_DATA:
    if err = d.Decode(res); err != nil {
    return err
    }
    case iproto.IPROTO_ERROR:
    if errorExtendedInfo, err = decodeBoxError(d); err != nil {
    return err
    }
    case iproto.IPROTO_ERROR_24:
    if resp.Error, err = d.DecodeString(); err != nil {
    return err
    }
    case iproto.IPROTO_SQL_INFO:
    if err = d.Decode(&resp.SQLInfo); err != nil {
    return err
    }
    case iproto.IPROTO_METADATA:
    if err = d.Decode(&resp.MetaData); err != nil {
    return err
    }
    case iproto.IPROTO_POSITION:
    if resp.Pos, err = d.DecodeBytes(); err != nil {
    return fmt.Errorf("unable to decode a position: %w", err)
    }
    default:
    if err = d.Skip(); err != nil {
    return err
    }
    }
    }
    if resp.Code != OkCode && resp.Code != PushCode {
    resp.Code &^= uint32(iproto.IPROTO_TYPE_ERROR)
    err = Error{iproto.Error(resp.Code), resp.Error, errorExtendedInfo}
    }
    }
    return
    }
./response.go:288:19: func literal escapes to heap

Some other allocations happens inside of msgpack package:

  1. In https://github.com/vmihailenco/msgpack/blob/0ac2a568aee92ef815c8d3e06b3fb3e2f79c18f6/decode.go#L84-L88
./decode.go:85:10: new(Decoder) escapes to heap
  1. In https://github.com/vmihailenco/msgpack/blob/0ac2a568aee92ef815c8d3e06b3fb3e2f79c18f6/decode.go#L617-L624
./decode.go:624:12: make([]byte, 0, 64) escapes to heap
  1. In https://github.com/vmihailenco/msgpack/blob/0ac2a568aee92ef815c8d3e06b3fb3e2f79c18f6/decode_string.go#L54-L60
./decode_string.go:59:16: string(b) escapes to heap
  1. In https://github.com/vmihailenco/msgpack/blob/0ac2a568aee92ef815c8d3e06b3fb3e2f79c18f6/decode_map.go#L72-L88
./decode_map.go:87:31: unexpectedCodeError{...} escapes to heap

Rest of the allocations (4) I could not find, perhaps they are somewhere deep inside of the msgpack calls.
Or there might be some allocations tied to use of interface{} as function arguments (Values stored in interfaces might be arbitrarily large, but only one word is dedicated to holding the value in the interface structure, so the assignment allocates a chunk of memory on the heap and records the pointer in the one-word slot. from https://research.swtch.com/interfaces). And we use interfaces a lot in the functions. But this is not detected by the go build -gcflags='-m=3' output.

@oleg-jukovec
Copy link
Collaborator Author

You could also try to run test with memory profiler. See -memprofile option to ensure that rest of allocation belongs to msgpack:

https://habr.com/en/companies/badoo/articles/301990/

@DerekBum
Copy link

Updated list of all 15 allocations:

  1. 2 allocations for creating a channel here

    events := make(chan connWatchEvent, 1024)

  2. 1 allocation while passing an argument to a goroutine function

    go conn.eventer(events)

  3. 1 allocation when creating a pointer to a Response struct

    resp := &Response{buf: smallBuf{b: respBytes}}

  4. 2 allocations while calling NewFuture

    fut := NewFuture()

    go-tarantool/future.go

    Lines 120 to 127 in d8df65d

    // NewFuture creates a new empty Future.
    func NewFuture() (fut *Future) {
    fut = &Future{}
    fut.ready = make(chan struct{}, 1000000000)
    fut.done = make(chan struct{})
    fut.pushes = make([]*Response, 0)
    return fut
    }

./future.go:122:8: &Future{} escapes to heap
./future.go:125:19: make([]*Response, 0) escapes to heap
  1. 2 allocations while calling NewSelectRequest

    go-tarantool/request.go

    Lines 862 to 874 in d8df65d

    // NewSelectRequest returns a new empty SelectRequest.
    func NewSelectRequest(space interface{}) *SelectRequest {
    req := new(SelectRequest)
    req.rtype = iproto.IPROTO_SELECT
    req.setSpace(space)
    req.isIteratorSet = false
    req.fetchPos = false
    req.iterator = IterAll
    req.key = []interface{}{}
    req.after = nil
    req.limit = 0xFFFFFFFF
    return req
    }
./request.go:864:12: new(SelectRequest) escapes to heap
./request.go:870:25: []interface {}{} escapes to heap
  1. 1 allocation in In

    go-tarantool/connection.go

    Lines 1244 to 1267 in d8df65d

    func read(r io.Reader, lenbuf []byte) (response []byte, err error) {
    var length int
    if _, err = io.ReadFull(r, lenbuf); err != nil {
    return
    }
    if lenbuf[0] != 0xce {
    err = errors.New("wrong response header")
    return
    }
    length = (int(lenbuf[1]) << 24) +
    (int(lenbuf[2]) << 16) +
    (int(lenbuf[3]) << 8) +
    int(lenbuf[4])
    if length == 0 {
    err = errors.New("response should not be 0 length")
    return
    }
    response = make([]byte, length)
    _, err = io.ReadFull(r, response)
    return
    }
./connection.go:1263:17: make([]byte, length) escapes to heap
  1. 2 allocations: 1 for creating a []byte array and 1 for creating a pointer to Response structure:
    shard.buf.b = make([]byte, 0, 128)

    resp := &Response{
./connection.go:1102:21: make([]byte, 0, 128) escapes to heap
./connection.go:1139:12: &Response{...} escapes to heap:

Some other allocations happens inside of msgpack package:

  1. 1 allocation in https://github.com/vmihailenco/msgpack/blob/0ac2a568aee92ef815c8d3e06b3fb3e2f79c18f6/decode.go#L84-L88
./decode.go:85:10: new(Decoder) escapes to heap
  1. 1 allocation in https://github.com/vmihailenco/msgpack/blob/0ac2a568aee92ef815c8d3e06b3fb3e2f79c18f6/decode.go#L617-L624
./decode.go:624:12: make([]byte, 0, 64) escapes to heap
  1. 1 allocation in https://github.com/vmihailenco/msgpack/blob/0ac2a568aee92ef815c8d3e06b3fb3e2f79c18f6/decode_string.go#L54-L60
./decode_string.go:59:16: string(b) escapes to heap
  1. 1 allocation in https://github.com/vmihailenco/msgpack/blob/0ac2a568aee92ef815c8d3e06b3fb3e2f79c18f6/decode_map.go#L72-L88
./decode_map.go:87:31: unexpectedCodeError{...} escapes to heap

15 allocations total.

@oleg-jukovec
Copy link
Collaborator Author

Updated list of all 15 allocations:

1. 2 allocations for creating a channel here https://github.com/tarantool/go-tarantool/blob/d8df65dcd0f29a6a5d07472115cbcf2753b12609/connection.go#L879

2. 1 allocation while passing an argument to a goroutine function https://github.com/tarantool/go-tarantool/blob/d8df65dcd0f29a6a5d07472115cbcf2753b12609/connection.go#L882

It does not look like a per request allocations. But let stop here. Thank you!

@oleg-jukovec oleg-jukovec changed the title v2: minimize allocations v3: minimize allocations Feb 12, 2024
@oleg-jukovec oleg-jukovec added v3 and removed v2 labels Feb 12, 2024
@KaymeKaydex
Copy link

I also want to note that msgpack supports zero resource allocation with sync.Pool, I think you can try using this implementation for decoding

code from msgpack

var decPool = sync.Pool{
	New: func() interface{} {
		return NewDecoder(nil)
	},
}

func GetDecoder() *Decoder {
	return decPool.Get().(*Decoder)
}

func PutDecoder(dec *Decoder) {
	dec.r = nil
	dec.s = nil
	decPool.Put(dec)
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants