All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] libuuid: improve uuid_unparse() performance
@ 2020-03-26 14:38 Aurelien LAJOIE
  2020-03-31 13:30 ` Karel Zak
  2020-04-01  8:31 ` Karel Zak
  0 siblings, 2 replies; 7+ messages in thread
From: Aurelien LAJOIE @ 2020-03-26 14:38 UTC (permalink / raw)
  To: util-linux; +Cc: Aurelien LAJOIE

There is 2 improvements:

 * remove useless uuid_unpack,
 * directly print the hexa format from memory without using printf
   we can do this as the bytes order is the network byte order
   https://tools.ietf.org/html/rfc4122#section-4.1.2
   even the spatially unique node identifier(the last 6 bytes)

The improvement is important, some results for 1000000 uuid_unparse calls:

Little Endian Ubuntu:
before took 382623 us
after  took  36740 us

Big Endian OpenBSD:
before took 3138172 us
after  took  180116 us

Signed-off-by: Aurelien LAJOIE <orel@melix.net>
---
 libuuid/src/unparse.c | 45 ++++++++++++++++++++-----------------------
 1 file changed, 21 insertions(+), 24 deletions(-)

diff --git a/libuuid/src/unparse.c b/libuuid/src/unparse.c
index a95bbb042..0e7e8eae4 100644
--- a/libuuid/src/unparse.c
+++ b/libuuid/src/unparse.c
@@ -36,41 +36,38 @@
 
 #include "uuidP.h"
 
-static const char *fmt_lower =
-	"%08x-%04x-%04x-%02x%02x-%02x%02x%02x%02x%02x%02x";
+static char const hexdigits_lower[16] = "0123456789abcdef";
+static char const hexdigits_upper[16] = "0123456789ABCDEF";
 
-static const char *fmt_upper =
-	"%08X-%04X-%04X-%02X%02X-%02X%02X%02X%02X%02X%02X";
-
-#ifdef UUID_UNPARSE_DEFAULT_UPPER
-#define FMT_DEFAULT fmt_upper
-#else
-#define FMT_DEFAULT fmt_lower
-#endif
-
-static void uuid_unparse_x(const uuid_t uu, char *out, const char *fmt)
+static void uuid_fmt(const uuid_t uuid, char *buf, char const fmt[restrict])
 {
-	struct uuid uuid;
+	char *p = buf;
 
-	uuid_unpack(uu, &uuid);
-	sprintf(out, fmt,
-		uuid.time_low, uuid.time_mid, uuid.time_hi_and_version,
-		uuid.clock_seq >> 8, uuid.clock_seq & 0xFF,
-		uuid.node[0], uuid.node[1], uuid.node[2],
-		uuid.node[3], uuid.node[4], uuid.node[5]);
+	for (int i = 0; i < 16; i++) {
+		if (i == 4 || i == 6 || i == 8 || i == 10) {
+			*p++ = '-';
+		}
+		size_t tmp = uuid[i];
+		*p++ = fmt[tmp >> 4];
+		*p++ = fmt[tmp & 15];
+	}
+	*p = '\0';
 }
 
 void uuid_unparse_lower(const uuid_t uu, char *out)
 {
-	uuid_unparse_x(uu, out,	fmt_lower);
+	uuid_fmt(uu, out, hexdigits_lower);
 }
 
 void uuid_unparse_upper(const uuid_t uu, char *out)
 {
-	uuid_unparse_x(uu, out,	fmt_upper);
+	uuid_fmt(uu, out, hexdigits_upper);
 }
 
+#ifdef UUID_UNPARSE_DEFAULT_UPPER
 void uuid_unparse(const uuid_t uu, char *out)
-{
-	uuid_unparse_x(uu, out, FMT_DEFAULT);
-}
+	__attribute__((alias("uuid_unparse_upper")));
+#else
+void uuid_unparse(const uuid_t uu, char *out)
+	__attribute__((alias("uuid_unparse_lower")));
+#endif
-- 
2.20.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] libuuid: improve uuid_unparse() performance
  2020-03-26 14:38 [PATCH v2] libuuid: improve uuid_unparse() performance Aurelien LAJOIE
@ 2020-03-31 13:30 ` Karel Zak
  2020-04-01  8:31 ` Karel Zak
  1 sibling, 0 replies; 7+ messages in thread
From: Karel Zak @ 2020-03-31 13:30 UTC (permalink / raw)
  To: Aurelien LAJOIE; +Cc: util-linux

On Thu, Mar 26, 2020 at 03:38:27PM +0100, Aurelien LAJOIE wrote:
>  libuuid/src/unparse.c | 45 ++++++++++++++++++++-----------------------
>  1 file changed, 21 insertions(+), 24 deletions(-)

Applied, thanks.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] libuuid: improve uuid_unparse() performance
  2020-03-26 14:38 [PATCH v2] libuuid: improve uuid_unparse() performance Aurelien LAJOIE
  2020-03-31 13:30 ` Karel Zak
@ 2020-04-01  8:31 ` Karel Zak
  2020-04-01  9:34   ` Peter Cordes
  1 sibling, 1 reply; 7+ messages in thread
From: Karel Zak @ 2020-04-01  8:31 UTC (permalink / raw)
  To: Aurelien LAJOIE; +Cc: util-linux

On Thu, Mar 26, 2020 at 03:38:27PM +0100, Aurelien LAJOIE wrote:
> +#ifdef UUID_UNPARSE_DEFAULT_UPPER
>  void uuid_unparse(const uuid_t uu, char *out)
> -{
> -	uuid_unparse_x(uu, out, FMT_DEFAULT);
> -}
> +	__attribute__((alias("uuid_unparse_upper")));
> +#else
> +void uuid_unparse(const uuid_t uu, char *out)
> +	__attribute__((alias("uuid_unparse_lower")));
> +#endif

The alias attribute is not portable to clang on Darwin.

 https://travis-ci.org/github/karelzak/util-linux/jobs/669244356?utm_medium=notification&utm_source=email

I have replaced the aliases with function calls.

 https://github.com/karelzak/util-linux/commit/bee464006776203a8cb545a35c86234181c7a55a

 Karel


-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] libuuid: improve uuid_unparse() performance
  2020-04-01  8:31 ` Karel Zak
@ 2020-04-01  9:34   ` Peter Cordes
  2020-04-01 10:00     ` Karel Zak
  0 siblings, 1 reply; 7+ messages in thread
From: Peter Cordes @ 2020-04-01  9:34 UTC (permalink / raw)
  To: Karel Zak; +Cc: Aurelien LAJOIE, util-linux

On Wed, Apr 1, 2020 at 5:31 AM Karel Zak <kzak@redhat.com> wrote:
>
> On Thu, Mar 26, 2020 at 03:38:27PM +0100, Aurelien LAJOIE wrote:
> > +     __attribute__((alias("uuid_unparse_upper")));
> > +#else
> > +void uuid_unparse(const uuid_t uu, char *out)
> > +     __attribute__((alias("uuid_unparse_lower")));
> > +#endif
>
> The alias attribute is not portable to clang on Darwin.
>
>  https://travis-ci.org/github/karelzak/util-linux/jobs/669244356?utm_medium=notification&utm_source=email
>
> I have replaced the aliases with function calls.
>
>  https://github.com/karelzak/util-linux/commit/bee464006776203a8cb545a35c86234181c7a55a

Since Darwin does support weak_alias, can we just use that?  Is it
"strong enough"?  I'm assuming nothing will override the alias so it
can still have the same efficient end result, having both symbols
resolve to the same address with no extra per-call overhead.  (And
without bloating the library from a compiler deciding to inline the
static helper function into all 3 instead of 2 callers.  Although most
programs probably only ever call one, so only one has to stay hot in
I-cache, with the others only bloating the file size and space in that
iTLB memory page.)

Or can we put the wrapper function in a .h so it can inline away?
That would bake the choice into applications that use libuuid, so you
couldn't change it just by rebuilding libuuid.  That's perhaps not
desirable; if applications wanted to have that choice baked in they
could have called the explicit upper or lower versions.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] libuuid: improve uuid_unparse() performance
  2020-04-01  9:34   ` Peter Cordes
@ 2020-04-01 10:00     ` Karel Zak
  2020-04-01 10:05       ` Karel Zak
  0 siblings, 1 reply; 7+ messages in thread
From: Karel Zak @ 2020-04-01 10:00 UTC (permalink / raw)
  To: Peter Cordes; +Cc: Aurelien LAJOIE, util-linux

On Wed, Apr 01, 2020 at 06:34:06AM -0300, Peter Cordes wrote:
> On Wed, Apr 1, 2020 at 5:31 AM Karel Zak <kzak@redhat.com> wrote:
> >
> > On Thu, Mar 26, 2020 at 03:38:27PM +0100, Aurelien LAJOIE wrote:
> > > +     __attribute__((alias("uuid_unparse_upper")));
> > > +#else
> > > +void uuid_unparse(const uuid_t uu, char *out)
> > > +     __attribute__((alias("uuid_unparse_lower")));
> > > +#endif
> >
> > The alias attribute is not portable to clang on Darwin.
> >
> >  https://travis-ci.org/github/karelzak/util-linux/jobs/669244356?utm_medium=notification&utm_source=email
> >
> > I have replaced the aliases with function calls.
> >
> >  https://github.com/karelzak/util-linux/commit/bee464006776203a8cb545a35c86234181c7a55a
> 
> Since Darwin does support weak_alias, can we just use that?  Is it
> "strong enough"?  I'm assuming nothing will override the alias so it
> can still have the same efficient end result, having both symbols
> resolve to the same address with no extra per-call overhead.  (And
> without bloating the library from a compiler deciding to inline the
> static helper function into all 3 instead of 2 callers.  Although most
> programs probably only ever call one, so only one has to stay hot in
> I-cache, with the others only bloating the file size and space in that
> iTLB memory page.)
> 
> Or can we put the wrapper function in a .h so it can inline away?
> That would bake the choice into applications that use libuuid, so you
> couldn't change it just by rebuilding libuuid.  That's perhaps not
> desirable; if applications wanted to have that choice baked in they
> could have called the explicit upper or lower versions.
 
 Frankly, what we're trying to fix by the alias? It sounds like
 premature optimization. The current solution works, maybe foo(bar())
 is also optimized by compiler. I have doubts that use inline function 
 in header or so makes a real sense.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] libuuid: improve uuid_unparse() performance
  2020-04-01 10:00     ` Karel Zak
@ 2020-04-01 10:05       ` Karel Zak
  2020-04-01 11:09         ` Peter Cordes
  0 siblings, 1 reply; 7+ messages in thread
From: Karel Zak @ 2020-04-01 10:05 UTC (permalink / raw)
  To: Peter Cordes; +Cc: Aurelien LAJOIE, util-linux

On Wed, Apr 01, 2020 at 12:00:09PM +0200, Karel Zak wrote:
> On Wed, Apr 01, 2020 at 06:34:06AM -0300, Peter Cordes wrote:
> > Or can we put the wrapper function in a .h so it can inline away?
> > That would bake the choice into applications that use libuuid, so you
> > couldn't change it just by rebuilding libuuid.  That's perhaps not
> > desirable; if applications wanted to have that choice baked in they
> > could have called the explicit upper or lower versions.
>  
>  Frankly, what we're trying to fix by the alias? It sounds like
>  premature optimization. The current solution works, maybe foo(bar())
>  is also optimized by compiler. I have doubts that use inline function 
>  in header or so makes a real sense.

BTW, would be better to make uuid_fmt() as inline function as we use
it in uuid_unparse_lower(), uuid_unparse_upper() and uuid_unparse()?

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2] libuuid: improve uuid_unparse() performance
  2020-04-01 10:05       ` Karel Zak
@ 2020-04-01 11:09         ` Peter Cordes
  0 siblings, 0 replies; 7+ messages in thread
From: Peter Cordes @ 2020-04-01 11:09 UTC (permalink / raw)
  To: Karel Zak; +Cc: Aurelien LAJOIE, util-linux

> Frankly, what we're trying to fix by the alias? It sounds like
 premature optimization.

Reducing static code size / file size of the library by ~190 bytes or
something like that IIRC for x86-64 for the current version, by
avoiding a 3rd duplicate of the code (if the compiler inlines
uuid_fmt()).  Or reducing the cost of forcing it to inline for cases
where the compiler chooses not to.

On Wed, Apr 1, 2020 at 7:06 AM Karel Zak <kzak@redhat.com> wrote:
> BTW, would be better to make uuid_fmt() as inline function as we use
> it in uuid_unparse_lower(), uuid_unparse_upper() and uuid_unparse()?
>

It's already  static.  The compiler is completely free to inline it
into both (or all 3) callsites, depending on tuning heuristics.  If we
expect that only one of the 3 will normally be "hot" at the same time,
then yeah forcing the issue with __attribute__((always_inline)) to
make sure we don't get a jmp tailcall is nice.

In practice for x86-64, gcc9 -O2 chooses not to inline because it's
fairly large.  So that would be good for programs that only ever call
uuid_unparse(), not a mix of upper/lower.

static inline would make zero difference, unless that keyword also
affects GCC's / clang's inlining heuristics as well as telling it that
a definition will be visible for any other compilation unit, so the
compiler doesn't need to emit a stand-alone definition if it inlines
it into all callers. (And nothing takes its address, etc.)

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-04-01 11:10 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-26 14:38 [PATCH v2] libuuid: improve uuid_unparse() performance Aurelien LAJOIE
2020-03-31 13:30 ` Karel Zak
2020-04-01  8:31 ` Karel Zak
2020-04-01  9:34   ` Peter Cordes
2020-04-01 10:00     ` Karel Zak
2020-04-01 10:05       ` Karel Zak
2020-04-01 11:09         ` Peter Cordes

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.