| Message ID | PAWPR08MB8982DC44B324DE1952A3EE97838EA@PAWPR08MB8982.eurprd08.prod.outlook.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v2,1/2] Add random benchmark | expand |
On 12/12/23 09:43, Wilco Dijkstra wrote: > +#include <single-thread.h> > #include <limits.h> > #include <stddef.h> > #include <stdlib.h> > @@ -288,6 +289,12 @@ __random (void) > { > int32_t retval; > > + if (SINGLE_THREAD_P) Since random.c is sort-of shared with gnulib, would there be any harm in using the user-visible "#include <sys/single_threaded.h>" and __libc_single_threaded instead? That would lessen the differences between the two source files. (I don't know why glibc code sometimes includes <sys/single_threaded.h> and sometimes <single-thread.h>.)
* Paul Eggert: > On 12/12/23 09:43, Wilco Dijkstra wrote: >> +#include <single-thread.h> >> #include <limits.h> >> #include <stddef.h> >> #include <stdlib.h> >> @@ -288,6 +289,12 @@ __random (void) >> { >> int32_t retval; >> + if (SINGLE_THREAD_P) > > Since random.c is sort-of shared with gnulib, would there be any harm > in using the user-visible "#include <sys/single_threaded.h>" and > __libc_single_threaded instead? That would lessen the differences > between the two source files. (I don't know why glibc code sometimes > includes <sys/single_threaded.h> and sometimes <single-thread.h>.) We can put the whole optimization into an #ifdef _LIBC block. SINGLE_THREAD_P is more optimized than the global __libc_single_threaded variable, and the difference is likely going to be visible on some architectures on some architectures. Thanks, Florian
Hi, >> Since random.c is sort-of shared with gnulib, would there be any harm >> in using the user-visible "#include <sys/single_threaded.h>" and >> __libc_single_threaded instead? That would lessen the differences >> between the two source files. (I don't know why glibc code sometimes >> includes <sys/single_threaded.h> and sometimes <single-thread.h>.) Which header should be used is confusing indeed. It seems that sys/single_threaded.h is more frequently used, so I'll change it to that. > We can put the whole optimization into an #ifdef _LIBC block. > SINGLE_THREAD_P is more optimized than the global __libc_single_threaded > variable, and the difference is likely going to be visible on some > architectures on some architectures. Given there are many uses of SINGLE_THREAD_P scattered around GLIBC this should really be done in the header or alternatively gnulib could define its own SINGLE_THREAD_P (no idea how it works today given that stdio and malloc have many uses). Cheers, Wilco
On 12/12/23 15:47, Paul Eggert wrote: > Since random.c is sort-of shared with gnulib, would there be any harm > in using the user-visible "#include <sys/single_threaded.h>" and > __libc_single_threaded instead? That would lessen the differences > between the two source files. (I don't know why glibc code sometimes > includes <sys/single_threaded.h> and sometimes <single-thread.h>.) Paul, Please update glibc/SHARED-FILES if random.c is being used by gnulib. We actively use SHARED-FILES to ensure that gnulib code changes have copyright assignment to the FSF. This is explained to new contributors via: https://sourceware.org/glibc/wiki/Contribution%20checklist#Copyright_FSF_or_disclaimer Please also note that it is not clear to me that all of the copyright in random.c is assigned to the FSF given the origins of that code.
diff --git a/stdlib/random.c b/stdlib/random.c index 62f22fac8d58c7977f09c134bf80a797750da645..a22de60a0f96031c74dd5a949b6717c2b0fc321a 100644 --- a/stdlib/random.c +++ b/stdlib/random.c @@ -51,6 +51,7 @@ SUCH DAMAGE.*/ #include <libc-lock.h> +#include <single-thread.h> #include <limits.h> #include <stddef.h> #include <stdlib.h> @@ -288,6 +289,12 @@ __random (void) { int32_t retval; + if (SINGLE_THREAD_P) + { + (void) __random_r (&unsafe_state, &retval); + return retval; + } + __libc_lock_lock (lock); (void) __random_r (&unsafe_state, &retval);