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

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

 



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@xxxxxxxxxx    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

Attachment: signature.asc
Description: OpenPGP digital signature

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]