[Maria-developers] bzr commit into MariaDB 5.1, with Maria 1.5:maria branch (knielsen:2703)
#At lp:maria 2703 knielsen@knielsen-hq.org 2009-06-10 Fix XtraDB to build with atomic operations, for good performance. The root of the problem is that ./configure mixed together two different things. One is the availability of GCC atomic operation intrinsics. The other is the selection of which primitives to use for my_atomic implementation. Then at some point a hack was made to not use GCC intrinsics in my_atomic to work around some test failures. But because the two things are mixed in ./configure, this as a side effect also makes GCC intrinsics unavailable for XtraDB. Fixed by splitting this in two in configure, so that we have HAVE_GCC_ATOMIC_BUILTINS for GCC intrinsics availability and MY_ATOMIC_MODE_GCC_BUILTINS for use in my_atomic. modified: configure.in include/atomic/nolock.h === modified file 'configure.in' --- a/configure.in 2009-04-23 13:06:16 +0000 +++ b/configure.in 2009-06-10 09:13:53 +0000 @@ -1726,6 +1726,30 @@ then fi fi +AC_CACHE_CHECK([whether the compiler provides atomic builtins], + [mysql_cv_gcc_atomic_builtins], [AC_TRY_RUN([ + int main() + { + int foo= -10; int bar= 10; + if (!__sync_fetch_and_add(&foo, bar) || foo) + return -1; + bar= __sync_lock_test_and_set(&foo, bar); + if (bar || foo != 10) + return -1; + bar= __sync_val_compare_and_swap(&bar, foo, 15); + if (bar) + return -1; + return 0; + } +], [mysql_cv_gcc_atomic_builtins=yes], + [mysql_cv_gcc_atomic_builtins=no], + [mysql_cv_gcc_atomic_builtins=no])]) + +if test "x$mysql_cv_gcc_atomic_builtins" = xyes; then + AC_DEFINE(HAVE_GCC_ATOMIC_BUILTINS, 1, + [Define to 1 if compiler provides atomic builtins.]) +fi + AC_ARG_WITH([atomic-ops], AC_HELP_STRING([--with-atomic-ops=rwlocks|smp|up], [Implement atomic operations using pthread rwlocks or atomic CPU @@ -1739,28 +1763,9 @@ case "$with_atomic_ops" in [Use pthread rwlocks for atomic ops]) ;; "smp") ;; "") - AC_CACHE_CHECK([whether the compiler provides atomic builtins], - [mysql_cv_gcc_atomic_builtins], [AC_TRY_RUN([ - int main() - { - int foo= -10; int bar= 10; - if (!__sync_fetch_and_add(&foo, bar) || foo) - return -1; - bar= __sync_lock_test_and_set(&foo, bar); - if (bar || foo != 10) - return -1; - bar= __sync_val_compare_and_swap(&bar, foo, 15); - if (bar) - return -1; - return 0; - } - ], [mysql_cv_gcc_atomic_builtins=yes_but_disabled], - [mysql_cv_gcc_atomic_builtins=no], - [mysql_cv_gcc_atomic_builtins=no])]) - - if test "x$mysql_cv_gcc_atomic_builtins" = xyes; then - AC_DEFINE(HAVE_GCC_ATOMIC_BUILTINS, 1, - [Define to 1 if compiler provides atomic builtins.]) + if test "x$mysql_cv_gcc_atomic_builtins" = xyes_but_disabled; then + AC_DEFINE([MY_ATOMIC_MODE_GCC_BUILTINS], [1], + [Use GCC atomic builtins for atomic ops]) fi ;; *) AC_MSG_ERROR(["$with_atomic_ops" is not a valid value for --with-atomic-ops]) ;; === modified file 'include/atomic/nolock.h' --- a/include/atomic/nolock.h 2008-02-05 15:47:11 +0000 +++ b/include/atomic/nolock.h 2009-06-10 09:13:53 +0000 @@ -14,7 +14,7 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */ #if defined(__i386__) || defined(_MSC_VER) || \ - defined(__x86_64__) || defined(HAVE_GCC_ATOMIC_BUILTINS) + defined(__x86_64__) || defined(MY_ATOMIC_MODE_GCC_BUILTINS) # ifdef MY_ATOMIC_MODE_DUMMY # define LOCK_prefix "" @@ -22,7 +22,7 @@ # define LOCK_prefix "lock" # endif -# ifdef HAVE_GCC_ATOMIC_BUILTINS +# ifdef MY_ATOMIC_MODE_GCC_BUILTINS # include "gcc_builtins.h" # elif __GNUC__ # include "x86-gcc.h"
Hi Serg, We discussed something like this on IRC some time ago. What do you think of this change? It keeps the use of GCC stuff for maria/my_atomic disabled, while still letting other parts (ie. XtraDB) check for GCC intrinsics availability. - Kristian. knielsen@knielsen-hq.org writes:
#At lp:maria
2703 knielsen@knielsen-hq.org 2009-06-10 Fix XtraDB to build with atomic operations, for good performance.
The root of the problem is that ./configure mixed together two different things. One is the availability of GCC atomic operation intrinsics. The other is the selection of which primitives to use for my_atomic implementation.
Then at some point a hack was made to not use GCC intrinsics in my_atomic to work around some test failures. But because the two things are mixed in ./configure, this as a side effect also makes GCC intrinsics unavailable for XtraDB.
Fixed by splitting this in two in configure, so that we have HAVE_GCC_ATOMIC_BUILTINS for GCC intrinsics availability and MY_ATOMIC_MODE_GCC_BUILTINS for use in my_atomic. modified: configure.in include/atomic/nolock.h
=== modified file 'configure.in' --- a/configure.in 2009-04-23 13:06:16 +0000 +++ b/configure.in 2009-06-10 09:13:53 +0000 @@ -1726,6 +1726,30 @@ then fi fi
+AC_CACHE_CHECK([whether the compiler provides atomic builtins], + [mysql_cv_gcc_atomic_builtins], [AC_TRY_RUN([ + int main() + { + int foo= -10; int bar= 10; + if (!__sync_fetch_and_add(&foo, bar) || foo) + return -1; + bar= __sync_lock_test_and_set(&foo, bar); + if (bar || foo != 10) + return -1; + bar= __sync_val_compare_and_swap(&bar, foo, 15); + if (bar) + return -1; + return 0; + } +], [mysql_cv_gcc_atomic_builtins=yes], + [mysql_cv_gcc_atomic_builtins=no], + [mysql_cv_gcc_atomic_builtins=no])]) + +if test "x$mysql_cv_gcc_atomic_builtins" = xyes; then + AC_DEFINE(HAVE_GCC_ATOMIC_BUILTINS, 1, + [Define to 1 if compiler provides atomic builtins.]) +fi + AC_ARG_WITH([atomic-ops], AC_HELP_STRING([--with-atomic-ops=rwlocks|smp|up], [Implement atomic operations using pthread rwlocks or atomic CPU @@ -1739,28 +1763,9 @@ case "$with_atomic_ops" in [Use pthread rwlocks for atomic ops]) ;; "smp") ;; "") - AC_CACHE_CHECK([whether the compiler provides atomic builtins], - [mysql_cv_gcc_atomic_builtins], [AC_TRY_RUN([ - int main() - { - int foo= -10; int bar= 10; - if (!__sync_fetch_and_add(&foo, bar) || foo) - return -1; - bar= __sync_lock_test_and_set(&foo, bar); - if (bar || foo != 10) - return -1; - bar= __sync_val_compare_and_swap(&bar, foo, 15); - if (bar) - return -1; - return 0; - } - ], [mysql_cv_gcc_atomic_builtins=yes_but_disabled], - [mysql_cv_gcc_atomic_builtins=no], - [mysql_cv_gcc_atomic_builtins=no])]) - - if test "x$mysql_cv_gcc_atomic_builtins" = xyes; then - AC_DEFINE(HAVE_GCC_ATOMIC_BUILTINS, 1, - [Define to 1 if compiler provides atomic builtins.]) + if test "x$mysql_cv_gcc_atomic_builtins" = xyes_but_disabled; then + AC_DEFINE([MY_ATOMIC_MODE_GCC_BUILTINS], [1], + [Use GCC atomic builtins for atomic ops]) fi ;; *) AC_MSG_ERROR(["$with_atomic_ops" is not a valid value for --with-atomic-ops]) ;;
=== modified file 'include/atomic/nolock.h' --- a/include/atomic/nolock.h 2008-02-05 15:47:11 +0000 +++ b/include/atomic/nolock.h 2009-06-10 09:13:53 +0000 @@ -14,7 +14,7 @@ Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA */
#if defined(__i386__) || defined(_MSC_VER) || \ - defined(__x86_64__) || defined(HAVE_GCC_ATOMIC_BUILTINS) + defined(__x86_64__) || defined(MY_ATOMIC_MODE_GCC_BUILTINS)
# ifdef MY_ATOMIC_MODE_DUMMY # define LOCK_prefix "" @@ -22,7 +22,7 @@ # define LOCK_prefix "lock" # endif
-# ifdef HAVE_GCC_ATOMIC_BUILTINS +# ifdef MY_ATOMIC_MODE_GCC_BUILTINS # include "gcc_builtins.h" # elif __GNUC__ # include "x86-gcc.h"
_______________________________________________ Mailing list: https://launchpad.net/~maria-developers Post to : maria-developers@lists.launchpad.net Unsubscribe : https://launchpad.net/~maria-developers More help : https://help.launchpad.net/ListHelp
Hi, Kristian! On Jun 10, Kristian Nielsen wrote:
Hi Serg,
We discussed something like this on IRC some time ago. What do you think of this change? It keeps the use of GCC stuff for maria/my_atomic disabled, while still letting other parts (ie. XtraDB) check for GCC intrinsics availability.
Looks fine. (disclaimer: I didn't vdiff the chunks to see if you copy-pasted correctly :)
2703 knielsen@knielsen-hq.org 2009-06-10 Fix XtraDB to build with atomic operations, for good performance.
Regards / Mit vielen Grüßen, Sergei -- __ ___ ___ ____ __ / |/ /_ __/ __/ __ \/ / Sergei Golubchik <serg@sun.com> / /|_/ / // /\ \/ /_/ / /__ Principal Software Engineer/Server Architect /_/ /_/\_, /___/\___\_\___/ Sun Microsystems GmbH, HRB München 161028 <___/ Sonnenallee 1, 85551 Kirchheim-Heimstetten Geschäftsführer: Thomas Schroeder, Wolfgang Engels, Wolf Frenkel Vorsitzender des Aufsichtsrates: Martin Häring
Hi!
"knielsen" == knielsen <knielsen@knielsen-hq.org> writes:
knielsen> #At lp:maria knielsen> 2703 knielsen@knielsen-hq.org 2009-06-10 knielsen> Fix XtraDB to build with atomic operations, for good performance. knielsen> The root of the problem is that ./configure mixed together two different things. One is the knielsen> availability of GCC atomic operation intrinsics. The other is the selection of which knielsen> primitives to use for my_atomic implementation. knielsen> Then at some point a hack was made to not use GCC intrinsics in my_atomic to work around knielsen> some test failures. But because the two things are mixed in ./configure, this as a side knielsen> effect also makes GCC intrinsics unavailable for XtraDB. knielsen> Fixed by splitting this in two in configure, so that we have HAVE_GCC_ATOMIC_BUILTINS for knielsen> GCC intrinsics availability and MY_ATOMIC_MODE_GCC_BUILTINS for use in my_atomic. knielsen> modified: knielsen> configure.in knielsen> include/atomic/nolock.h knielsen> === modified file 'configure.in' knielsen> --- a/configure.in 2009-04-23 13:06:16 +0000 knielsen> +++ b/configure.in 2009-06-10 09:13:53 +0000 knielsen> @@ -1726,6 +1726,30 @@ then knielsen> fi knielsen> fi knielsen> +AC_CACHE_CHECK([whether the compiler provides atomic builtins], knielsen> + [mysql_cv_gcc_atomic_builtins], [AC_TRY_RUN([ knielsen> + int main() knielsen> + { knielsen> + int foo= -10; int bar= 10; knielsen> + if (!__sync_fetch_and_add(&foo, bar) || foo) knielsen> + return -1; knielsen> + bar= __sync_lock_test_and_set(&foo, bar); knielsen> + if (bar || foo != 10) knielsen> + return -1; knielsen> + bar= __sync_val_compare_and_swap(&bar, foo, 15); knielsen> + if (bar) knielsen> + return -1; knielsen> + return 0; knielsen> + } knielsen> +], [mysql_cv_gcc_atomic_builtins=yes], knielsen> + [mysql_cv_gcc_atomic_builtins=no], knielsen> + [mysql_cv_gcc_atomic_builtins=no])]) knielsen> + knielsen> +if test "x$mysql_cv_gcc_atomic_builtins" = xyes; then knielsen> + AC_DEFINE(HAVE_GCC_ATOMIC_BUILTINS, 1, knielsen> + [Define to 1 if compiler provides atomic builtins.]) knielsen> +fi knielsen> + knielsen> AC_ARG_WITH([atomic-ops], knielsen> AC_HELP_STRING([--with-atomic-ops=rwlocks|smp|up], knielsen> [Implement atomic operations using pthread rwlocks or atomic CPU knielsen> @@ -1739,28 +1763,9 @@ case "$with_atomic_ops" in knielsen> [Use pthread rwlocks for atomic ops]) ;; knielsen> "smp") ;; knielsen> "") knielsen> - AC_CACHE_CHECK([whether the compiler provides atomic builtins], knielsen> - [mysql_cv_gcc_atomic_builtins], [AC_TRY_RUN([ knielsen> - int main() knielsen> - { knielsen> - int foo= -10; int bar= 10; knielsen> - if (!__sync_fetch_and_add(&foo, bar) || foo) knielsen> - return -1; knielsen> - bar= __sync_lock_test_and_set(&foo, bar); knielsen> - if (bar || foo != 10) knielsen> - return -1; knielsen> - bar= __sync_val_compare_and_swap(&bar, foo, 15); knielsen> - if (bar) knielsen> - return -1; knielsen> - return 0; knielsen> - } knielsen> - ], [mysql_cv_gcc_atomic_builtins=yes_but_disabled], knielsen> - [mysql_cv_gcc_atomic_builtins=no], knielsen> - [mysql_cv_gcc_atomic_builtins=no])]) knielsen> - knielsen> - if test "x$mysql_cv_gcc_atomic_builtins" = xyes; then knielsen> - AC_DEFINE(HAVE_GCC_ATOMIC_BUILTINS, 1, knielsen> - [Define to 1 if compiler provides atomic builtins.]) knielsen> + if test "x$mysql_cv_gcc_atomic_builtins" = xyes_but_disabled; then knielsen> + AC_DEFINE([MY_ATOMIC_MODE_GCC_BUILTINS], [1], knielsen> + [Use GCC atomic builtins for atomic ops]) <cut> Under which circumstances could the last 'if' be true? The new code don't seem to be able to set mysql_cv_gcc_atomic_builtins to yes_but_disabled which makes the last if always false and we never define MY_ATOMIC_MODE_GCC_BUILTINS as 1. Regards, Monty
Michael Widenius <monty@askmonty.org> writes:
knielsen> === modified file 'configure.in' knielsen> --- a/configure.in 2009-04-23 13:06:16 +0000 knielsen> +++ b/configure.in 2009-06-10 09:13:53 +0000
knielsen> @@ -1739,28 +1763,9 @@ case "$with_atomic_ops" in knielsen> [Use pthread rwlocks for atomic ops]) ;; knielsen> "smp") ;; knielsen> "") knielsen> - AC_CACHE_CHECK([whether the compiler provides atomic builtins], knielsen> - [mysql_cv_gcc_atomic_builtins], [AC_TRY_RUN([ knielsen> - int main() knielsen> - { knielsen> - int foo= -10; int bar= 10; knielsen> - if (!__sync_fetch_and_add(&foo, bar) || foo) knielsen> - return -1; knielsen> - bar= __sync_lock_test_and_set(&foo, bar); knielsen> - if (bar || foo != 10) knielsen> - return -1; knielsen> - bar= __sync_val_compare_and_swap(&bar, foo, 15); knielsen> - if (bar) knielsen> - return -1; knielsen> - return 0; knielsen> - } knielsen> - ], [mysql_cv_gcc_atomic_builtins=yes_but_disabled], knielsen> - [mysql_cv_gcc_atomic_builtins=no], knielsen> - [mysql_cv_gcc_atomic_builtins=no])]) knielsen> - knielsen> - if test "x$mysql_cv_gcc_atomic_builtins" = xyes; then knielsen> - AC_DEFINE(HAVE_GCC_ATOMIC_BUILTINS, 1, knielsen> - [Define to 1 if compiler provides atomic builtins.]) knielsen> + if test "x$mysql_cv_gcc_atomic_builtins" = xyes_but_disabled; then knielsen> + AC_DEFINE([MY_ATOMIC_MODE_GCC_BUILTINS], [1], knielsen> + [Use GCC atomic builtins for atomic ops])
Under which circumstances could the last 'if' be true? The new code don't seem to be able to set mysql_cv_gcc_atomic_builtins to yes_but_disabled which makes the last if always false and we never define MY_ATOMIC_MODE_GCC_BUILTINS as 1.
You are right that the if can never be true. However, this is unchanged behaviour from before the patch, check the line with [mysql_cv_gcc_atomic_builtins=yes_but_disabled] above. Serg changed the "yes" to "yes_but_disabled" some time ago in the Maria tree. This is a hack to disable the use of GCC intrinsics in my_atomic. Serg told me that the reason is that there were failures in the unit tests when using GCC intrinsics. I did try to run unit tests with this hack removed, but I was not able to repeat any failures. I do think we should remove the hack and instead fix the real issue, if it is still there. However, I wanted to keep this separate from the XtraDB merge, which is quite complicated enough without introducing additional unrelated issues. - Kristian.
participants (4)
-
knielsen＠knielsen-hq.org -
Kristian Nielsen
-
Michael Widenius -
Sergei Golubchik