[libvirt] [PATCH] rpc: RH1026137: Fix slow volume download (virsh vol-download)

[Martin Kletzander]

On Mon, Jul 20, 2015 at 05:42:11PM +0300, Ossi Herrala wrote:
>> On Sat, Jun 06, 2015 at 07:36:48PM +0000, Ossi Herrala wrote:
>>
>> Sorry to miss this mail, it got buried somehow and I haven't got to it
>> until now since nobody pinged it.  Sorry for the long wait then.
>>
>
>No worries and thank you for taking time to review my patch. See new
>patch attached as well as comments on the memory usage below.
>
>The patch is tested on latest master (e46791e003444ce825feaf5bb2a16f778ee951e5).
>
>
>> The only thing I would mention wrt to how it works after this patch is
>> that it will consume some memory that's not needed, precisely
>> (sizeof(struct iovec) + sizeof(void *)) * unreadMBs.  It might be
>> worth it to do:
>>
>>  memmove(st->incomingVec, st->incomingVec + st->readVec,
>>          st->writeVec - st->readVec);
>>  VIR_SHRINK_N(st->incomingVec, st->readVec);
>>  st->writeVec -= st->readVec;
>>  st->readVec = 0;
>>
>> Apart from that it's definitely *way* better approach.  What do you
>> think of such modification?  This would go either instead
>> 'st->readVec++', but rather at the end of this function, so it's not
>> done after each MB read.
>>
>
>I totally agree and implemented freeing of the memory in the new
>patch:
>
>  if (st->readVec >= 16) {
>    memmove(st->incomingVec, st->incomingVec + st->readVec,
>            sizeof(*st->incomingVec)*(st->writeVec - st->readVec));
>    VIR_SHRINK_N(st->incomingVec, st->writeVec, st->readVec);
>    VIR_DEBUG("shrink removed %zu, left %zu", st->readVec, st->writeVec);
>    st->readVec = 0;
>  }
>
>now it only frees memory in chunks of 16 to avoid doing memmove() all
>the time. The iovec is 16 bytes (in 64 bit Linux) so this frees 256
>bytes at a time and in my testing usually everything or almost
>everything that is allocated.
>
>
>Thanks again for taking time to review and commit, and I hope the new
>patch below is formatted ok.
>

I managed to apply it as needed.  I like how it works now and the
feeing is fine, too.  I tested it and it seems it's working perfectly.
Without this patch I've got stuck on 1MiB/s locally after 11GiB in
circa 1 minute, with this patch I managed to download 20GiB in less
than a minute.

ACK && will push after release since we're way after rc2 already.

Again, sorry for the delays and thanks for the contribution!

Have a nice day,
Martin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20150803/7b76425d/attachment-0001.sig>