From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:45746) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bN4KL-0003av-NP for qemu-devel@nongnu.org; Tue, 12 Jul 2016 16:30:46 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bN4KJ-0002ZN-Kt for qemu-devel@nongnu.org; Tue, 12 Jul 2016 16:30:44 -0400 References: <1468345431-106198-1-git-send-email-vsementsov@virtuozzo.com> <57853A35.4030501@redhat.com> <578540F4.2070309@virtuozzo.com> From: Eric Blake Message-ID: <5785536A.2050409@redhat.com> Date: Tue, 12 Jul 2016 14:30:34 -0600 MIME-Version: 1.0 In-Reply-To: <578540F4.2070309@virtuozzo.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="LIdH6etsgO2exqxp7QxHSpc1ad9EK4ofN" Subject: Re: [Qemu-devel] [PATCH] qcow2: do not allocate extra memory List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Vladimir Sementsov-Ogievskiy , qemu-devel@nongnu.org, qemu-block@nongnu.org Cc: mreitz@redhat.com, kwolf@redhat.com, stefanha@redhat.com, den@openvz.org, fabrice@bellard.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --LIdH6etsgO2exqxp7QxHSpc1ad9EK4ofN From: Eric Blake To: Vladimir Sementsov-Ogievskiy , qemu-devel@nongnu.org, qemu-block@nongnu.org Cc: mreitz@redhat.com, kwolf@redhat.com, stefanha@redhat.com, den@openvz.org, fabrice@bellard.org Message-ID: <5785536A.2050409@redhat.com> Subject: Re: [PATCH] qcow2: do not allocate extra memory References: <1468345431-106198-1-git-send-email-vsementsov@virtuozzo.com> <57853A35.4030501@redhat.com> <578540F4.2070309@virtuozzo.com> In-Reply-To: <578540F4.2070309@virtuozzo.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable On 07/12/2016 01:11 PM, Vladimir Sementsov-Ogievskiy wrote: > On 12.07.2016 21:43, Eric Blake wrote: >> On 07/12/2016 11:43 AM, Vladimir Sementsov-Ogievskiy wrote: >>> There are no needs to allocate more than one cluster, as we set >>> avail_out for deflate to one cluster. >>> >>> ... >>> strm.avail_out =3D s->cluster_size; >>> strm.next_out =3D out_buf; >>> >>> ret =3D deflate(&strm, Z_FINISH); >>> ... >>> out_len =3D strm.next_out - out_buf; >> You've skipped what is done with ret, which will be different accordin= g >> to whether the entire compressed stream fit in the buffer described by= >> strm, and that would have to be audited as part of your proposed patch= =2E >=20 > ret would be Z_STREAM_END if it fit in and Z_OK if not. (if there are n= o > errors ofcourse). What I've skipped? I just say that nobody knows about= > this extra allocation - neither zlib nor other code in this function > (except g_free=3D). Okay, I've thought about this a bit more, and chatted with John on IRC. It looks like the slop is indeed wasted. And while I don't know that performance will be noticeably better, I _do_ think you are correct that readability is easier to understand without the slop. And who knows - for a malloc() implementation that uses mmap for large requests, and rounds requests up to page multiples, malloc(64k) may indeed be a more efficient use of memory than malloc(64k+slop), which has to burn an entire page for memory that is never touched. So my end conclusion is that I'd like the commit message to be a bit more comprehensive (include some of your arguments made in the follow up messages, such as the fact that we correctly handle Z_STREAM_END vs. Z_OK in deciding whether to go with a compressed cluster in the first place), but the idea itself is sane. I'll give R-b to a v2, but not right now, because I want to make sure the final commit message is sufficient to avoid another hour of digging through RFC and zlib documentation when it gets revisited down the road. --=20 Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org --LIdH6etsgO2exqxp7QxHSpc1ad9EK4ofN Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 Comment: Public key at http://people.redhat.com/eblake/eblake.gpg Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/ iQEcBAEBCAAGBQJXhVNqAAoJEKeha0olJ0NqzZYIAK1mY5lSR5hDXFDAZOEZz9CD 99qDAv+JnvvW2BEr3t0Si5PzvMGJmqLWZwfcmssoftZpkx92lNHlNrkdvqQxIWbt evLyH8yF/SHytAU5RwKgn1+FJv9XAJrNLY0DBGStBXybYOLmJYDq8cDHA9DfAy4m TtRL6fTxWUazMEso0NhVuyvhcZg1zECWv4aIFkKjL2ajvsjv9P42Db2K+FT1Ojz0 7a5umiYzYt1iua04dwvy36aWH8tnCOOeW3apfFpc/6ZTW2K3K0q4loXswkuSi39P 3TAeXOMsxQ1ue/Z4SMchxNhXrZohn+9NkjxG1WwPyoSSmK60OIRLaSZ3ZuiIoMw= =WxgU -----END PGP SIGNATURE----- --LIdH6etsgO2exqxp7QxHSpc1ad9EK4ofN--