[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]