planetscale / vtprotobuf

A Protocol Buffers compiler that generates optimized marshaling & unmarshaling Go code for ProtoBuf APIv2
BSD 3-Clause "New" or "Revised" License
868 stars 74 forks source link

Question about behavior of `ReturnToVTPool()` and `Reset()` #64

Open joe-elliott opened 1 year ago

joe-elliott commented 1 year ago

Using this proto:

message Parent {
  option (vtproto.mempool) = true;
  repeated Child children = 1;
  Child one = 2;
}

message Child {
  option (vtproto.mempool) = true;
  uint32 field = 1;
}

When calling ReturnToVTPool() on Parent it calls ResetVT on all children and then calls m.Reset()

func (m *Parent) ResetVT() {
    for _, mm := range m.Children {
        mm.ResetVT()
    }
    m.One.ReturnToVTPool()
    m.Reset()
}

However m.Reset() allocates a new object and overwrites the existing object entirely:

func (x *Parent) Reset() {
    *x = Parent{}

This nils out all fields on the parent throwing away the slice for the GC to handle. Am I missing something? Is there some way to put back into the pool, call ResetVT() but not call Reset()?

npordash commented 1 year ago

I'm fairly certain it is a bug. I commented about this in https://github.com/planetscale/vtprotobuf/pull/35#issuecomment-1133384043 since I saw this behavior when trying to compile using the v0.3.0 tag. I'm still using v0.2.0 right now because of it.

cyriltovena commented 1 year ago

I believe the solution is to use a fallthrough here https://github.com/planetscale/vtprotobuf/blob/96ede25e29a0325741fd05d6e9e48364d65ea137/features/pool/pool.go#L64C8-L64C8