On Sat, Oct 30, 2021 at 09:41:11PM +0300, Nir Soffer wrote:
> On Sat, Oct 30, 2021 at 8:30 PM Richard W.M. Jones <[email protected]> wrote:
> >
> > On Sat, Oct 30, 2021 at 07:56:05PM +0300, Nir Soffer wrote:
> > > The generic vector reallocs on every append. Add benchmarks to measure
> > > the cost with uint32 vector (used for copying extents) and the effect of
> > > reserving space upfront.
> >
> > Hmm, so it does.  I'd actually forgotten it was this bad, and I agree
> > with introducing the factor in patch 2.
> >
> > But for this particular patch, you'll realise when you manage CI that
> > having tests that just consume time makes things hard.  It only takes
> > 0.004 seconds on x86-64, but when you're running valgrind on armv7
> > under TCG etc it'll be another story.  So I'd prefer if the benchmark
> > was '#if 0' or perhaps conditional on an obscure environment variable
> > being set.
> 
> Make sense.
> 
> I think it will be easiest to put the benchmarks in a separate file, and run
> them using:
> 
>     make bench
> 
> What do you think?

Sure, similar to check-valgrind.

Rich.

> >
> > Rich.
> >
> > > The tests show that realloc is pretty efficient, but calling reserve
> > > before the appends speeds the appends up significantly.
> > >
> > > $ ./test-vector | grep bench
> > > bench_reserve: 1000000 appends in 0.004159 s
> > > bench_append: 1000000 appends in 0.014897 s
> > >
> > > Signed-off-by: Nir Soffer <[email protected]>
> > > ---
> > >  common/utils/Makefile.am   |  2 +-
> > >  common/utils/bench.h       | 72 ++++++++++++++++++++++++++++++++++++++
> > >  common/utils/test-vector.c | 49 ++++++++++++++++++++++++++
> > >  3 files changed, 122 insertions(+), 1 deletion(-)
> > >  create mode 100644 common/utils/bench.h
> > >
> > > diff --git a/common/utils/Makefile.am b/common/utils/Makefile.am
> > > index b273ada..4730cea 100644
> > > --- a/common/utils/Makefile.am
> > > +++ b/common/utils/Makefile.am
> > > @@ -59,6 +59,6 @@ test_human_size_SOURCES = test-human-size.c 
> > > human-size.c human-size.h
> > >  test_human_size_CPPFLAGS = -I$(srcdir)
> > >  test_human_size_CFLAGS = $(WARNINGS_CFLAGS)
> > >
> > > -test_vector_SOURCES = test-vector.c vector.c vector.h
> > > +test_vector_SOURCES = test-vector.c vector.c vector.h bench.h
> > >  test_vector_CPPFLAGS = -I$(srcdir)
> > >  test_vector_CFLAGS = $(WARNINGS_CFLAGS)
> > > diff --git a/common/utils/bench.h b/common/utils/bench.h
> > > new file mode 100644
> > > index 0000000..496a361
> > > --- /dev/null
> > > +++ b/common/utils/bench.h
> > > @@ -0,0 +1,72 @@
> > > +/* libnbd
> > > + * Copyright (C) 2021 Red Hat Inc.
> > > + *
> > > + * Redistribution and use in source and binary forms, with or without
> > > + * modification, are permitted provided that the following conditions are
> > > + * met:
> > > + *
> > > + * * Redistributions of source code must retain the above copyright
> > > + * notice, this list of conditions and the following disclaimer.
> > > + *
> > > + * * Redistributions in binary form must reproduce the above copyright
> > > + * notice, this list of conditions and the following disclaimer in the
> > > + * documentation and/or other materials provided with the distribution.
> > > + *
> > > + * * Neither the name of Red Hat nor the names of its contributors may be
> > > + * used to endorse or promote products derived from this software without
> > > + * specific prior written permission.
> > > + *
> > > + * THIS SOFTWARE IS PROVIDED BY RED HAT AND CONTRIBUTORS ''AS IS'' AND
> > > + * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO,
> > > + * THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A
> > > + * PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL RED HAT OR
> > > + * CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> > > + * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> > > + * LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF
> > > + * USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
> > > + * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
> > > + * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
> > > + * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
> > > + * SUCH DAMAGE.
> > > + */
> > > +
> > > +#ifndef LIBNBD_BENCH_H
> > > +#define LIBNBD_BENCH_H
> > > +
> > > +#include <sys/time.h>
> > > +
> > > +#define MICROSECONDS 1000000
> > > +
> > > +struct bench {
> > > +    struct timeval start, stop;
> > > +};
> > > +
> > > +static inline void
> > > +bench_start(struct bench *b)
> > > +{
> > > +  gettimeofday (&b->start, NULL);
> > > +}
> > > +
> > > +static inline void
> > > +bench_stop(struct bench *b)
> > > +{
> > > +  gettimeofday (&b->stop, NULL);
> > > +}
> > > +
> > > +static inline double
> > > +bench_sec(struct bench *b)
> > > +{
> > > +  struct timeval dt;
> > > +
> > > +  dt.tv_sec = b->stop.tv_sec - b->start.tv_sec;
> > > +  dt.tv_usec = b->stop.tv_usec - b->start.tv_usec;
> > > +
> > > +  if (dt.tv_usec < 0) {
> > > +    dt.tv_sec -= 1;
> > > +    dt.tv_usec += MICROSECONDS;
> > > +  }
> > > +
> > > +  return ((double)dt.tv_sec * MICROSECONDS + dt.tv_usec) / MICROSECONDS;
> > > +}
> > > +
> > > +#endif /* LIBNBD_BENCH_H */
> > > diff --git a/common/utils/test-vector.c b/common/utils/test-vector.c
> > > index 94b2aeb..28e882f 100644
> > > --- a/common/utils/test-vector.c
> > > +++ b/common/utils/test-vector.c
> > > @@ -38,9 +38,13 @@
> > >  #undef NDEBUG /* Keep test strong even for nbdkit built without 
> > > assertions */
> > >  #include <assert.h>
> > >
> > > +#include "bench.h"
> > >  #include "vector.h"
> > >
> > > +#define APPENDS 1000000
> > > +
> > >  DEFINE_VECTOR_TYPE(int64_vector, int64_t);
> > > +DEFINE_VECTOR_TYPE(uint32_vector, uint32_t);
> > >  DEFINE_VECTOR_TYPE(string_vector, char *);
> > >
> > >  static int
> > > @@ -113,10 +117,55 @@ test_string_vector (void)
> > >    free (v.ptr);
> > >  }
> > >
> > > +static void
> > > +bench_reserve (void)
> > > +{
> > > +  uint32_vector v = empty_vector;
> > > +  struct bench b;
> > > +  int64_t usec;
> > > +
> > > +  bench_start(&b);
> > > +
> > > +  uint32_vector_reserve(&v, APPENDS);
> > > +
> > > +  for (uint32_t i = 0; i < APPENDS; i++) {
> > > +    uint32_vector_append (&v, i);
> > > +  }
> > > +
> > > +  bench_stop(&b);
> > > +
> > > +  assert (v.ptr[APPENDS - 1] == APPENDS - 1);
> > > +  free (v.ptr);
> > > +
> > > +  printf ("bench_reserve: %d appends in %.6f s\n", APPENDS, bench_sec 
> > > (&b));
> > > +}
> > > +
> > > +static void
> > > +bench_append (void)
> > > +{
> > > +  uint32_vector v = empty_vector;
> > > +  struct bench b;
> > > +
> > > +  bench_start(&b);
> > > +
> > > +  for (uint32_t i = 0; i < APPENDS; i++) {
> > > +    uint32_vector_append (&v, i);
> > > +  }
> > > +
> > > +  bench_stop(&b);
> > > +
> > > +  assert (v.ptr[APPENDS - 1] == APPENDS - 1);
> > > +  free (v.ptr);
> > > +
> > > +  printf ("bench_append: %d appends in %.6f s\n", APPENDS, bench_sec 
> > > (&b));
> > > +}
> > > +
> > >  int
> > >  main (int argc, char *argv[])
> > >  {
> > >    test_int64_vector ();
> > >    test_string_vector ();
> > > +  bench_reserve ();
> > > +  bench_append ();
> > >    return 0;
> > >  }
> > > --
> > > 2.31.1
> >
> > --
> > Richard Jones, Virtualization Group, Red Hat 
> > http://people.redhat.com/~rjones
> > Read my programming and virtualization blog: http://rwmj.wordpress.com
> > Fedora Windows cross-compiler. Compile Windows programs, test, and
> > build Windows installers. Over 100 libraries supported.
> > http://fedoraproject.org/wiki/MinGW
> >

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-builder quickly builds VMs from scratch
http://libguestfs.org/virt-builder.1.html

_______________________________________________
Libguestfs mailing list
[email protected]
https://listman.redhat.com/mailman/listinfo/libguestfs

Reply via email to