On 03.05.2012 10:01, Daniel P. Berrange wrote:
On Wed, May 02, 2012 at 01:52:27PM -0600, Eric Blake wrote:
> On 04/27/2012 07:22 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 | 6 +-
>> src/rpc/virnetserverclient.c | 20 ++-
>> tests/virnetmessagetest.c | 393 +++++++++++++++++++++++-------------------
>> 5 files changed, 264 insertions(+), 183 deletions(-)
>
> I haven't looked closely at this, but I do have a generic question:
>
> Are you malloc'ing and free'ing the buffer for each rpc on every call,
> or are you tracking a pool of buffers and only alloc'ing when needed?
> Furthermore, can you use the stack or a statically allocated array to
> receive short messages, saving alloc only for the long messages? I'm
> worried about performance if we malloc on every single RPC call.
To be honest I'm not convinced malloc() would be a big factor in the
speed of the RPC calls. That said you could optimize it using the
VIR_ALLOC_VAR to allocate the virNetMessage struct + the buffer
in one go. ALlocate all messages with 256kb initially and then you
can relloc larger only if needed, and re-alloc back to 256 kb when
done.
Daniel
Okay, point taken. Although I'd rather avoid allocating virNetMessage
struct and buffer at once. It would complicate the code again, because
the client side code has this struct _virNetClient; which already
contains virNetMessage. However, if I'd need to re-alloc, I'd need to
realloc whole message (!) therefore change the member to
virNetMessagePtr. This implies too many changes.
Michal