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

> 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.