Re: [libvirt] [PATCH v2 1/2] rpc: Switch to dynamically allocated message buffer

On 05/15/2012 09:04 AM, Michal Privoznik wrote: > Currently, we are allocating buffer for RPC messages statically. > This is not such pain when RPC limits are small. However, if we want > ever to increase those limits, we need to allocate buffer dynamically, > based on RPC message len (= the first 4 bytes). Therefore we will > decrease our mem usage in most cases and still be flexible enough in > corner cases. > --- > src/rpc/virnetclient.c | 16 ++- > src/rpc/virnetmessage.c | 12 ++- > src/rpc/virnetmessage.h | 5 +- > src/rpc/virnetserverclient.c | 24 +++- > tests/virnetmessagetest.c | 393 +++++++++++++++++++++++------------------- > 5 files changed, 266 insertions(+), 184 deletions(-) In isolation, this patch should have no impact on RPC, while enabling the next patch. And your point of using less memory by default may have slight benefits. I still wonder if pre-allocating a default size (maybe 4k?), and only reallocating bigger as needed, followed by releasing back to the default size, may help reduce malloc() calls, but we'd need to benchmark that to know for sure, and I'm okay saving it for a separate patch. > @@ -1030,8 +1036,13 @@ virNetClientIOReadMessage(virNetClientPtr client) > ssize_t ret; > > /* Start by reading length word */ > - if (client->msg.bufferLength == 0) > + if (client->msg.bufferLength == 0) { > client->msg.bufferLength = 4; > + if (VIR_ALLOC_N(client->msg.buffer, client->msg.bufferLength) < 0) { > + virReportOOMError(); > + return -ENOMEM; > + } For example, this would be one location where default allocation of a larger size will avoid the churn of malloc'ing 4 bytes followed by realloc'ing to something bigger. ACK.