9:17 a.m.
This is largely a simple refactoring, but it combines another
definition of this function from virt-builder which had a slightly
different prototype.
---
builder/pxzcat-c.c | 20 +-------------------
daemon/daemon.h | 18 ------------------
lib/guestfs-internal-all.h | 18 ++++++++++++++++++
3 files changed, 19 insertions(+), 37 deletions(-)
diff --git a/builder/pxzcat-c.c b/builder/pxzcat-c.c
index 33923b532..ef37849ed 100644
--- a/builder/pxzcat-c.c
+++ b/builder/pxzcat-c.c
@@ -409,24 +409,6 @@ parse_indexes (value filenamev, int fd)
return combined_index;
}
-/* Return true iff the buffer is all zero bytes.
- *
- * Note that gcc is smart enough to optimize this properly:
- *
http://stackoverflow.com/questions/1493936/faster-means-of-checking-for-a...
- */
-static inline int
-is_zero (const unsigned char *buffer, size_t size)
-{
- size_t i;
-
- for (i = 0; i < size; ++i) {
- if (buffer[i] != 0)
- return 0;
- }
-
- return 1;
-}
-
struct global_state {
/* Current iterator. Threads update this, but it is protected by a
* mutex, and each thread takes a copy of it when working on it.
@@ -681,7 +663,7 @@ worker_thread (void *vp)
/* Don't write if the block is all zero, to preserve output file
* sparseness. However we have to update oposition.
*/
- if (!is_zero (outbuf, wsz)) {
+ if (!is_zero ((char *) outbuf, wsz)) {
if (xpwrite (global->ofd, outbuf, wsz, oposition) == -1) {
perror (global->outputfile);
return &state->status;
diff --git a/daemon/daemon.h b/daemon/daemon.h
index 332e2966a..a317139a5 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -327,24 +327,6 @@ extern void pulse_mode_cancel (void);
*/
extern void notify_progress_no_ratelimit (uint64_t position, uint64_t total, const struct
timeval *now);
-/* Return true iff the buffer is all zero bytes.
- *
- * Note that gcc is smart enough to optimize this properly:
- *
http://stackoverflow.com/questions/1493936/faster-means-of-checking-for-a...
- */
-static inline int
-is_zero (const char *buffer, size_t size)
-{
- size_t i;
-
- for (i = 0; i < size; ++i) {
- if (buffer[i] != 0)
- return 0;
- }
-
- return 1;
-}
-
/* Helper for functions that need a root filesystem mounted.
* NB. Cannot be used for FileIn functions.
*/
diff --git a/lib/guestfs-internal-all.h b/lib/guestfs-internal-all.h
index 0c841ff67..4f7433332 100644
--- a/lib/guestfs-internal-all.h
+++ b/lib/guestfs-internal-all.h
@@ -89,6 +89,24 @@
#define xdr_uint32_t xdr_u_int32_t
#endif
+/* Return true iff the buffer is all zero bytes.
+ *
+ * Note that gcc is smart enough to optimize this properly:
+ *
http://stackoverflow.com/questions/1493936/faster-means-of-checking-for-a...
+ */
+static inline int
+is_zero (const char *buffer, size_t size)
+{
+ size_t i;
+
+ for (i = 0; i < size; ++i) {
+ if (buffer[i] != 0)
+ return 0;
+ }
+
+ return 1;
+}
+
/* Macro which compiles the regexp once when the program/library is
* loaded, and frees it when the library is unloaded.
*/
--
2.12.0
9:35 a.m.
On 04/19/2017 09:17 AM, Richard W.M. Jones wrote:
This is largely a simple refactoring, but it combines another
definition of this function from virt-builder which had a slightly
different prototype.
---
+/* Return true iff the buffer is all zero bytes.
+ *
+ * Note that gcc is smart enough to optimize this properly:
+ *
http://stackoverflow.com/questions/1493936/faster-means-of-checking-for-a...
+ */
+static inline int
+is_zero (const char *buffer, size_t size)
+{
+ size_t i;
+
+ for (i = 0; i < size; ++i) {
+ if (buffer[i] != 0)
+ return 0;
+ }
+
+ return 1;
+}
This is still byte-at-a-time. Often, you can get even faster results by
exploiting libc's optimizations in memcmp (particularly when it is
comparing pointers with nice alignments), by manually checking that the
first 16 bytes are zero, then letting memcmp do the rest:
size_t i;
size_t limit = MIN(size, 16);
for (i = 0; i < limit; ++i)
if (buffer[i])
return 0;
if (size != limit)
return !memcmp(buffer, buffer + 16, size - 16);
return 1;
--
Eric Blake, Principal Software Engineer
Red Hat, Inc. +1-919-301-3266
Virtualization: qemu.org | libvirt.org
9:40 a.m.
On Wed, Apr 19, 2017 at 09:35:16AM -0500, Eric Blake wrote:
On 04/19/2017 09:17 AM, Richard W.M. Jones wrote:
> This is largely a simple refactoring, but it combines another
> definition of this function from virt-builder which had a slightly
> different prototype.
> ---
> +/* Return true iff the buffer is all zero bytes.
> + *
> + * Note that gcc is smart enough to optimize this properly:
> + *
http://stackoverflow.com/questions/1493936/faster-means-of-checking-for-a...
> + */
> +static inline int
> +is_zero (const char *buffer, size_t size)
> +{
> + size_t i;
> +
> + for (i = 0; i < size; ++i) {
> + if (buffer[i] != 0)
> + return 0;
> + }
> +
> + return 1;
> +}
This is still byte-at-a-time. Often, you can get even faster results by
exploiting libc's optimizations in memcmp (particularly when it is
comparing pointers with nice alignments), by manually checking that the
first 16 bytes are zero, then letting memcmp do the rest:
size_t i;
size_t limit = MIN(size, 16);
for (i = 0; i < limit; ++i)
if (buffer[i])
return 0;
if (size != limit)
return !memcmp(buffer, buffer + 16, size - 16);
return 1;
If performance is critical, QEMU has a permissively licensed impl that
uses sse/avx instructions to super-optimize it on x86 (util/bufferiszero.c).
It is a large amount of code though, so only worth it if this is a notable
bottleneck for libguestfs.
Regards,
Daniel
--
|: https://berrange.com -o- https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org -o- https://fstop138.berrange.com :|
|: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
10:50 a.m.
On Wed, Apr 19, 2017 at 03:40:49PM +0100, Daniel P. Berrange wrote:
On Wed, Apr 19, 2017 at 09:35:16AM -0500, Eric Blake wrote:
> On 04/19/2017 09:17 AM, Richard W.M. Jones wrote:
> > This is largely a simple refactoring, but it combines another
> > definition of this function from virt-builder which had a slightly
> > different prototype.
> > ---
>
> > +/* Return true iff the buffer is all zero bytes.
> > + *
> > + * Note that gcc is smart enough to optimize this properly:
> > + *
http://stackoverflow.com/questions/1493936/faster-means-of-checking-for-a...
> > + */
> > +static inline int
> > +is_zero (const char *buffer, size_t size)
> > +{
> > + size_t i;
> > +
> > + for (i = 0; i < size; ++i) {
> > + if (buffer[i] != 0)
> > + return 0;
> > + }
> > +
> > + return 1;
> > +}
>
> This is still byte-at-a-time.
RPM uses -mtune=generic, and GCC doesn't optimize the loop in any way.
Literally just iterating over bytes in the most naive way possible.
With -mtune=broadwell it unrolls the loop (16 times if I call it on a
fixed size buffer).
> Often, you can get even faster results by
> exploiting libc's optimizations in memcmp (particularly when it is
> comparing pointers with nice alignments), by manually checking that the
> first 16 bytes are zero, then letting memcmp do the rest:
>
> size_t i;
> size_t limit = MIN(size, 16);
> for (i = 0; i < limit; ++i)
> if (buffer[i])
> return 0;
> if (size != limit)
> return !memcmp(buffer, buffer + 16, size - 16);
> return 1;
This is a neat idea!
If performance is critical, QEMU has a permissively licensed impl
that
uses sse/avx instructions to super-optimize it on x86 (util/bufferiszero.c).
It is a large amount of code though, so only worth it if this is a notable
bottleneck for libguestfs.
This would be a lot easier if C defined this as a standard function ...
Rich.
--
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine. Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/
2774
days inactive
2774
days old
3 comments
3 participants
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Richard W.M. Jones