Reduce allocations with internal smart buffering

[willardf]

Been thinking about them allocations lately...

Sucks that we make a bunch of new byte[] every time we do a send. But it's also annoying to have to deal with threaded reliable resends making buffer lifetimes racy.

So I reinvented Smart Pointers and it seems like it works pretty well.

Edit: To avoid confusion: AddUsage is to be used right before a SmartBuffer needs to be preserved for some reason. This only happens in a few places: As we enqueue data for later, as we store data for resending, and as we call BeginSendTo. Nearly all usages of SmartBuffer don't need AddUsage or Recycle. A simple using statement wraps that up so we can never fail to recycle.

[willardf]

If my understanding is correct, my recommendation would be to remove the usagecount and AddUsage() method. It seemed to me the use cases here are relatively isolated and don't need to support multiple contexts holding references simultaneously?

MAYBE, but probably you are missing a scenario. The issue that causes all of this is:

UnreliableSend -> BufferCopy -> BeginSendTo -> EndSendTo -> Recycle
ReliableSend -> BufferCopy -> AttachReliableId -> RetrySending -> BeginSendTo -> EndSendTo -> NO Recycle
                                                                              -> BeginSendTo -> EndSendTo -> NO Recycle
                                                                                                                                 -> ReceiveAck -> Recycle

In particular, we don't have a good way to differentiate a recycling send and a non-recycling send. Listeners have trouble with this because we queue data to another thread. But it's simple enough, we could pass the send option through the stack. Could even be part of the SmartBuffer... /thinking But the scenario you might have missed is that we could be resending while we receive an ack. If we return the buffer we might send torn data. Buffers are shared across users, so it could even be a security issue.

That SAID! I think we can use this to simplify AddUsage. We don't need to use it all the time, just in PingPacket. Everywhere else should be allowed to get away with one Recycle and no AddUsage.

I will defend usageCount tho. Pooling has to be precise. That's really the problem with pooling. If you recycle twice, that's never not an issue. The only bad part is that I should have made it explode if we reach negative.

[willardf]

Yeah, this all looks good to me. I would want to put it through its paces with some stress tests and probably a canary deployment to West-1?

@willardf, My inclination is that this should go into both the AU client and the AU server simultaneously, but mebbe we could we roll it out in the server first and test?

Yeah, because the changes are all internal, it should be totally safe to do a server deployment first. Worth noting that we've been running this version of Hazel (non-DTLS) in REDACTED since we did the optimization pass. So while not the paces, it's at least been smoke tested.