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

[Michal Privoznik]

On 17.05.2012 00:11, Eric Blake wrote:
> 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.
> 


Agree definitely. I'll post follow up patch as soon as this is committed.