[v2] sysdeps: Add 'get_fast_jitter' interace in fast-jitter.h

Message ID 20220427201302.2525203-1-goldstein.w.n@gmail.com
State Committed
Commit 911c63a51c690dd1a97dfc587097277029baf00f
Headers
Series [v2] sysdeps: Add 'get_fast_jitter' interace in fast-jitter.h |

Checks

Context Check Description
dj/TryBot-apply_patch success Patch applied to master at the time it was sent
dj/TryBot-32bit success Build for i686

Commit Message

Noah Goldstein April 27, 2022, 8:13 p.m. UTC
  'get_fast_jitter' is meant to be used purely for performance
purposes. In all cases it's used it should be acceptable to get no
randomness (see default case). An example use case is in setting
jitter for retries between threads at a lock. There is a
performance benefit to having jitter, but only if the jitter can
be generated very quickly and ultimately there is no serious issue
if no jitter is generated.

The implementation generally uses 'HP_TIMING_NOW' iff it is
inlined (avoid any potential syscall paths).
---
 sysdeps/generic/fast-jitter.h | 42 +++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)
 create mode 100644 sysdeps/generic/fast-jitter.h
  

Comments

H.J. Lu April 27, 2022, 8:46 p.m. UTC | #1
On Wed, Apr 27, 2022 at 1:13 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
>
> 'get_fast_jitter' is meant to be used purely for performance
> purposes. In all cases it's used it should be acceptable to get no
> randomness (see default case). An example use case is in setting
> jitter for retries between threads at a lock. There is a
> performance benefit to having jitter, but only if the jitter can
> be generated very quickly and ultimately there is no serious issue
> if no jitter is generated.
>
> The implementation generally uses 'HP_TIMING_NOW' iff it is
> inlined (avoid any potential syscall paths).
> ---
>  sysdeps/generic/fast-jitter.h | 42 +++++++++++++++++++++++++++++++++++
>  1 file changed, 42 insertions(+)
>  create mode 100644 sysdeps/generic/fast-jitter.h
>
> diff --git a/sysdeps/generic/fast-jitter.h b/sysdeps/generic/fast-jitter.h
> new file mode 100644
> index 0000000000..4dd53e3475
> --- /dev/null
> +++ b/sysdeps/generic/fast-jitter.h
> @@ -0,0 +1,42 @@
> +/* Fallback for fast jitter just return 0.
> +   Copyright (C) 2019-2022 Free Software Foundation, Inc.
> +   This file is part of the GNU C Library.
> +
> +   The GNU C Library is free software; you can redistribute it and/or
> +   modify it under the terms of the GNU Lesser General Public
> +   License as published by the Free Software Foundation; either
> +   version 2.1 of the License, or (at your option) any later version.
> +
> +   The GNU C Library is distributed in the hope that it will be useful,
> +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> +   Lesser General Public License for more details.
> +
> +   You should have received a copy of the GNU Lesser General Public
> +   License along with the GNU C Library; if not, see
> +   <https://www.gnu.org/licenses/>.  */
> +
> +#ifndef _FAST_JITTER_H
> +# define _FAST_JITTER_H
> +
> +# include <stdint.h>
> +# include <hp-timing.h>
> +
> +/* Baseline just return 0.  We could create jitter using a clock or
> +   'random_bits' but that may imply a syscall and the goal of
> +   'get_fast_jitter' is minimal overhead "randomness" when such
> +   randomness helps performance.  Adding high overhead the function
> +   defeats the purpose.  */
> +static inline uint32_t
> +get_fast_jitter (void)
> +{
> +# if HP_TIMING_INLINE
> +  hp_timing_t jitter;
> +  HP_TIMING_NOW (jitter);
> +  return (uint32_t) jitter;
> +# else
> +  return 0;
> +# endif
> +}
> +
> +#endif
> --
> 2.25.1
>

LGTM.    An arch can provide a better one if needed.

Reviewed-by: H.J. Lu <hjl.tools@gmail.com>

Thanks.
  
Sunil Pandey Sept. 11, 2022, 8:27 p.m. UTC | #2
On Wed, Apr 27, 2022 at 1:47 PM H.J. Lu via Libc-alpha
<libc-alpha@sourceware.org> wrote:
>
> On Wed, Apr 27, 2022 at 1:13 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
> >
> > 'get_fast_jitter' is meant to be used purely for performance
> > purposes. In all cases it's used it should be acceptable to get no
> > randomness (see default case). An example use case is in setting
> > jitter for retries between threads at a lock. There is a
> > performance benefit to having jitter, but only if the jitter can
> > be generated very quickly and ultimately there is no serious issue
> > if no jitter is generated.
> >
> > The implementation generally uses 'HP_TIMING_NOW' iff it is
> > inlined (avoid any potential syscall paths).
> > ---
> >  sysdeps/generic/fast-jitter.h | 42 +++++++++++++++++++++++++++++++++++
> >  1 file changed, 42 insertions(+)
> >  create mode 100644 sysdeps/generic/fast-jitter.h
> >
> > diff --git a/sysdeps/generic/fast-jitter.h b/sysdeps/generic/fast-jitter.h
> > new file mode 100644
> > index 0000000000..4dd53e3475
> > --- /dev/null
> > +++ b/sysdeps/generic/fast-jitter.h
> > @@ -0,0 +1,42 @@
> > +/* Fallback for fast jitter just return 0.
> > +   Copyright (C) 2019-2022 Free Software Foundation, Inc.
> > +   This file is part of the GNU C Library.
> > +
> > +   The GNU C Library is free software; you can redistribute it and/or
> > +   modify it under the terms of the GNU Lesser General Public
> > +   License as published by the Free Software Foundation; either
> > +   version 2.1 of the License, or (at your option) any later version.
> > +
> > +   The GNU C Library is distributed in the hope that it will be useful,
> > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > +   Lesser General Public License for more details.
> > +
> > +   You should have received a copy of the GNU Lesser General Public
> > +   License along with the GNU C Library; if not, see
> > +   <https://www.gnu.org/licenses/>.  */
> > +
> > +#ifndef _FAST_JITTER_H
> > +# define _FAST_JITTER_H
> > +
> > +# include <stdint.h>
> > +# include <hp-timing.h>
> > +
> > +/* Baseline just return 0.  We could create jitter using a clock or
> > +   'random_bits' but that may imply a syscall and the goal of
> > +   'get_fast_jitter' is minimal overhead "randomness" when such
> > +   randomness helps performance.  Adding high overhead the function
> > +   defeats the purpose.  */
> > +static inline uint32_t
> > +get_fast_jitter (void)
> > +{
> > +# if HP_TIMING_INLINE
> > +  hp_timing_t jitter;
> > +  HP_TIMING_NOW (jitter);
> > +  return (uint32_t) jitter;
> > +# else
> > +  return 0;
> > +# endif
> > +}
> > +
> > +#endif
> > --
> > 2.25.1
> >
>
> LGTM.    An arch can provide a better one if needed.
>
> Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
>
> Thanks.
>
> --
> H.J.

I would like to backport this patch to release branch 2.33, 2.34 and 2.35

Any comments/suggestions or objections on this.

commit 911c63a51c690dd1a97dfc587097277029baf00f
Author: Noah Goldstein <goldstein.w.n@gmail.com>
Date:   Wed Apr 27 15:13:02 2022 -0500

    sysdeps: Add 'get_fast_jitter' interace in fast-jitter.h

    'get_fast_jitter' is meant to be used purely for performance
    purposes. In all cases it's used it should be acceptable to get no
    randomness (see default case). An example use case is in setting
    jitter for retries between threads at a lock. There is a
    performance benefit to having jitter, but only if the jitter can
    be generated very quickly and ultimately there is no serious issue
    if no jitter is generated.
  
Noah Goldstein Sept. 14, 2022, 1:26 a.m. UTC | #3
On Sun, Sep 11, 2022 at 1:27 PM Sunil Pandey <skpgkp2@gmail.com> wrote:
>
> On Wed, Apr 27, 2022 at 1:47 PM H.J. Lu via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
> >
> > On Wed, Apr 27, 2022 at 1:13 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
> > >
> > > 'get_fast_jitter' is meant to be used purely for performance
> > > purposes. In all cases it's used it should be acceptable to get no
> > > randomness (see default case). An example use case is in setting
> > > jitter for retries between threads at a lock. There is a
> > > performance benefit to having jitter, but only if the jitter can
> > > be generated very quickly and ultimately there is no serious issue
> > > if no jitter is generated.
> > >
> > > The implementation generally uses 'HP_TIMING_NOW' iff it is
> > > inlined (avoid any potential syscall paths).
> > > ---
> > >  sysdeps/generic/fast-jitter.h | 42 +++++++++++++++++++++++++++++++++++
> > >  1 file changed, 42 insertions(+)
> > >  create mode 100644 sysdeps/generic/fast-jitter.h
> > >
> > > diff --git a/sysdeps/generic/fast-jitter.h b/sysdeps/generic/fast-jitter.h
> > > new file mode 100644
> > > index 0000000000..4dd53e3475
> > > --- /dev/null
> > > +++ b/sysdeps/generic/fast-jitter.h
> > > @@ -0,0 +1,42 @@
> > > +/* Fallback for fast jitter just return 0.
> > > +   Copyright (C) 2019-2022 Free Software Foundation, Inc.
> > > +   This file is part of the GNU C Library.
> > > +
> > > +   The GNU C Library is free software; you can redistribute it and/or
> > > +   modify it under the terms of the GNU Lesser General Public
> > > +   License as published by the Free Software Foundation; either
> > > +   version 2.1 of the License, or (at your option) any later version.
> > > +
> > > +   The GNU C Library is distributed in the hope that it will be useful,
> > > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > > +   Lesser General Public License for more details.
> > > +
> > > +   You should have received a copy of the GNU Lesser General Public
> > > +   License along with the GNU C Library; if not, see
> > > +   <https://www.gnu.org/licenses/>.  */
> > > +
> > > +#ifndef _FAST_JITTER_H
> > > +# define _FAST_JITTER_H
> > > +
> > > +# include <stdint.h>
> > > +# include <hp-timing.h>
> > > +
> > > +/* Baseline just return 0.  We could create jitter using a clock or
> > > +   'random_bits' but that may imply a syscall and the goal of
> > > +   'get_fast_jitter' is minimal overhead "randomness" when such
> > > +   randomness helps performance.  Adding high overhead the function
> > > +   defeats the purpose.  */
> > > +static inline uint32_t
> > > +get_fast_jitter (void)
> > > +{
> > > +# if HP_TIMING_INLINE
> > > +  hp_timing_t jitter;
> > > +  HP_TIMING_NOW (jitter);
> > > +  return (uint32_t) jitter;
> > > +# else
> > > +  return 0;
> > > +# endif
> > > +}
> > > +
> > > +#endif
> > > --
> > > 2.25.1
> > >
> >
> > LGTM.    An arch can provide a better one if needed.
> >
> > Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
> >
> > Thanks.
> >
> > --
> > H.J.
>
> I would like to backport this patch to release branch 2.33, 2.34 and 2.35
>
> Any comments/suggestions or objections on this.

Fine by me.
>
> commit 911c63a51c690dd1a97dfc587097277029baf00f
> Author: Noah Goldstein <goldstein.w.n@gmail.com>
> Date:   Wed Apr 27 15:13:02 2022 -0500
>
>     sysdeps: Add 'get_fast_jitter' interace in fast-jitter.h
>
>     'get_fast_jitter' is meant to be used purely for performance
>     purposes. In all cases it's used it should be acceptable to get no
>     randomness (see default case). An example use case is in setting
>     jitter for retries between threads at a lock. There is a
>     performance benefit to having jitter, but only if the jitter can
>     be generated very quickly and ultimately there is no serious issue
>     if no jitter is generated.
  
Noah Goldstein Sept. 29, 2022, 12:11 a.m. UTC | #4
On Sun, Sep 11, 2022 at 4:27 PM Sunil Pandey <skpgkp2@gmail.com> wrote:
>
> On Wed, Apr 27, 2022 at 1:47 PM H.J. Lu via Libc-alpha
> <libc-alpha@sourceware.org> wrote:
> >
> > On Wed, Apr 27, 2022 at 1:13 PM Noah Goldstein <goldstein.w.n@gmail.com> wrote:
> > >
> > > 'get_fast_jitter' is meant to be used purely for performance
> > > purposes. In all cases it's used it should be acceptable to get no
> > > randomness (see default case). An example use case is in setting
> > > jitter for retries between threads at a lock. There is a
> > > performance benefit to having jitter, but only if the jitter can
> > > be generated very quickly and ultimately there is no serious issue
> > > if no jitter is generated.
> > >
> > > The implementation generally uses 'HP_TIMING_NOW' iff it is
> > > inlined (avoid any potential syscall paths).
> > > ---
> > >  sysdeps/generic/fast-jitter.h | 42 +++++++++++++++++++++++++++++++++++
> > >  1 file changed, 42 insertions(+)
> > >  create mode 100644 sysdeps/generic/fast-jitter.h
> > >
> > > diff --git a/sysdeps/generic/fast-jitter.h b/sysdeps/generic/fast-jitter.h
> > > new file mode 100644
> > > index 0000000000..4dd53e3475
> > > --- /dev/null
> > > +++ b/sysdeps/generic/fast-jitter.h
> > > @@ -0,0 +1,42 @@
> > > +/* Fallback for fast jitter just return 0.
> > > +   Copyright (C) 2019-2022 Free Software Foundation, Inc.
> > > +   This file is part of the GNU C Library.
> > > +
> > > +   The GNU C Library is free software; you can redistribute it and/or
> > > +   modify it under the terms of the GNU Lesser General Public
> > > +   License as published by the Free Software Foundation; either
> > > +   version 2.1 of the License, or (at your option) any later version.
> > > +
> > > +   The GNU C Library is distributed in the hope that it will be useful,
> > > +   but WITHOUT ANY WARRANTY; without even the implied warranty of
> > > +   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> > > +   Lesser General Public License for more details.
> > > +
> > > +   You should have received a copy of the GNU Lesser General Public
> > > +   License along with the GNU C Library; if not, see
> > > +   <https://www.gnu.org/licenses/>.  */
> > > +
> > > +#ifndef _FAST_JITTER_H
> > > +# define _FAST_JITTER_H
> > > +
> > > +# include <stdint.h>
> > > +# include <hp-timing.h>
> > > +
> > > +/* Baseline just return 0.  We could create jitter using a clock or
> > > +   'random_bits' but that may imply a syscall and the goal of
> > > +   'get_fast_jitter' is minimal overhead "randomness" when such
> > > +   randomness helps performance.  Adding high overhead the function
> > > +   defeats the purpose.  */
> > > +static inline uint32_t
> > > +get_fast_jitter (void)
> > > +{
> > > +# if HP_TIMING_INLINE
> > > +  hp_timing_t jitter;
> > > +  HP_TIMING_NOW (jitter);
> > > +  return (uint32_t) jitter;
> > > +# else
> > > +  return 0;
> > > +# endif
> > > +}
> > > +
> > > +#endif
> > > --
> > > 2.25.1
> > >
> >
> > LGTM.    An arch can provide a better one if needed.
> >
> > Reviewed-by: H.J. Lu <hjl.tools@gmail.com>
> >
> > Thanks.
> >
> > --
> > H.J.
>
> I would like to backport this patch to release branch 2.33, 2.34 and 2.35

Fine by me.
>
> Any comments/suggestions or objections on this.
>
> commit 911c63a51c690dd1a97dfc587097277029baf00f
> Author: Noah Goldstein <goldstein.w.n@gmail.com>
> Date:   Wed Apr 27 15:13:02 2022 -0500
>
>     sysdeps: Add 'get_fast_jitter' interace in fast-jitter.h
>
>     'get_fast_jitter' is meant to be used purely for performance
>     purposes. In all cases it's used it should be acceptable to get no
>     randomness (see default case). An example use case is in setting
>     jitter for retries between threads at a lock. There is a
>     performance benefit to having jitter, but only if the jitter can
>     be generated very quickly and ultimately there is no serious issue
>     if no jitter is generated.
  

Patch

diff --git a/sysdeps/generic/fast-jitter.h b/sysdeps/generic/fast-jitter.h
new file mode 100644
index 0000000000..4dd53e3475
--- /dev/null
+++ b/sysdeps/generic/fast-jitter.h
@@ -0,0 +1,42 @@ 
+/* Fallback for fast jitter just return 0.
+   Copyright (C) 2019-2022 Free Software Foundation, Inc.
+   This file is part of the GNU C Library.
+
+   The GNU C Library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Lesser General Public
+   License as published by the Free Software Foundation; either
+   version 2.1 of the License, or (at your option) any later version.
+
+   The GNU C Library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Lesser General Public License for more details.
+
+   You should have received a copy of the GNU Lesser General Public
+   License along with the GNU C Library; if not, see
+   <https://www.gnu.org/licenses/>.  */
+
+#ifndef _FAST_JITTER_H
+# define _FAST_JITTER_H
+
+# include <stdint.h>
+# include <hp-timing.h>
+
+/* Baseline just return 0.  We could create jitter using a clock or
+   'random_bits' but that may imply a syscall and the goal of
+   'get_fast_jitter' is minimal overhead "randomness" when such
+   randomness helps performance.  Adding high overhead the function
+   defeats the purpose.  */
+static inline uint32_t
+get_fast_jitter (void)
+{
+# if HP_TIMING_INLINE
+  hp_timing_t jitter;
+  HP_TIMING_NOW (jitter);
+  return (uint32_t) jitter;
+# else
+  return 0;
+# endif
+}
+
+#endif