From mboxrd@z Thu Jan 1 00:00:00 1970 From: Mathieu Othacehe Subject: bug#26441: =?UTF-8?Q?Gnulib=E2=80=99s_?= =?UTF-8?Q?=E2=80=98test-lock=E2=80=99?= fails to complete on machines with >= 32 cores Date: Thu, 13 Apr 2017 08:39:55 +0200 Message-ID: <86poggbykk.fsf@gmail.com> References: <871sszuznt.fsf@gnu.org> <86shleatk3.fsf@gmail.com> <87wpapdedp.fsf@gnu.org> <2008088.TpEHzIiNi8@omega> <87h91td2s5.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" Return-path: Received: from eggs.gnu.org ([2001:4830:134:3::10]:57149) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1cyYQH-0006Z7-CB for bug-guix@gnu.org; Thu, 13 Apr 2017 02:40:07 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1cyYQE-0004gB-65 for bug-guix@gnu.org; Thu, 13 Apr 2017 02:40:05 -0400 Received: from debbugs.gnu.org ([208.118.235.43]:46793) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1cyYQE-0004g3-1q for bug-guix@gnu.org; Thu, 13 Apr 2017 02:40:02 -0400 Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1cyYQD-0004gf-NE for bug-guix@gnu.org; Thu, 13 Apr 2017 02:40:01 -0400 Sender: "Debbugs-submit" Resent-Message-ID: In-reply-to: <87h91td2s5.fsf@gnu.org> List-Id: Bug reports for GNU Guix List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-guix-bounces+gcggb-bug-guix=m.gmane.org@gnu.org Sender: "bug-Guix" To: Ludovic =?UTF-8?Q?Court=C3=A8s?= , 26441@debbugs.gnu.org --=-=-= Content-Type: text/plain Hi, Here's a patch for gettext, to solve test-lock performance issues. While rebuilding all gettext dependencies, I noticed that there are other programs impacted by this bug : * libunistring * coreutils * findutils But the list may be bigger : http://git.gnu.org.ua/cgit/gnulib.git/tree/users.txt Those users are embedding gnulib source code (via git submodules for example), so I'm not sure what to do here ... We can consider that it's not such a big deal and ignore this problem waiting for programs to integrate the fix, or we can patch them on core-updates. WDYT ? Thank you, Mathieu --=-=-= Content-Type: text/x-diff; charset=utf-8 Content-Disposition: inline; filename=0001-gnu-gettext-Fix-make-check-issues-on-multi-core-mach.patch Content-Transfer-Encoding: 8bit >From e4ad9aa61fa75afa4417616de36cd25e9631fd67 Mon Sep 17 00:00:00 2001 From: Mathieu Othacehe Date: Wed, 12 Apr 2017 21:17:24 +0200 Subject: [PATCH] gnu: gettext: Fix make check issues on multi-core machines. * gnu/packages/patches/gettext-gnulib-multi-core.patch: New file. * gnu/packages/patches/gettext-multi-core.patch: New file. * gnu/packages/gettext.scm (gettext-minimal)[patches]: Add a reference to the two previous patches. --- gnu/packages/gettext.scm | 9 +- .../patches/gettext-gnulib-multi-core.patch | 176 ++++++++++++++++++++ gnu/packages/patches/gettext-multi-core.patch | 183 +++++++++++++++++++++ 3 files changed, 367 insertions(+), 1 deletion(-) create mode 100644 gnu/packages/patches/gettext-gnulib-multi-core.patch create mode 100644 gnu/packages/patches/gettext-multi-core.patch diff --git a/gnu/packages/gettext.scm b/gnu/packages/gettext.scm index f583d1c2c..2a749e3a6 100644 --- a/gnu/packages/gettext.scm +++ b/gnu/packages/gettext.scm @@ -5,6 +5,7 @@ ;;; Copyright © 2016 Efraim Flashner ;;; Copyright © 2016 Alex Kost ;;; Copyright © 2017 Marius Bakke +;;; Copyright © 2017 Mathieu Othacehe ;;; ;;; This file is part of GNU Guix. ;;; @@ -45,7 +46,13 @@ version ".tar.gz")) (sha256 (base32 - "0hsw28f9q9xaggjlsdp2qmbp2rbd1mp0njzan2ld9kiqwkq2m57z")))) + "0hsw28f9q9xaggjlsdp2qmbp2rbd1mp0njzan2ld9kiqwkq2m57z")) + ;; test-lock has performance issues on multi-core machines, + ;; it hangs or takes a long time to complete. + ;; There is one commit in gettext and one commit + ;; in gettext's embedded gnulib to fix this issue. + (patches (search-patches "gettext-multi-core.patch" + "gettext-gnulib-multi-core.patch")))) (build-system gnu-build-system) (outputs '("out" "doc")) ;8 MiB of HTML diff --git a/gnu/packages/patches/gettext-gnulib-multi-core.patch b/gnu/packages/patches/gettext-gnulib-multi-core.patch new file mode 100644 index 000000000..4393f3a1b --- /dev/null +++ b/gnu/packages/patches/gettext-gnulib-multi-core.patch @@ -0,0 +1,176 @@ +This patch fixes performance problems on multi-core machines. +See commit 480d374e596a0ee3fed168ab42cd84c313ad3c89 in gnulib +from Bruno Haible . + +diff --git a/gettext-tools/gnulib-tests/test-lock.c b/gettext-tools/gnulib-tests/test-lock.c +index cb734b4e6..aa6de2739 100644 +--- a/gettext-tools/gnulib-tests/test-lock.c ++++ b/gettext-tools/gnulib-tests/test-lock.c +@@ -50,6 +50,13 @@ + Uncomment this to see if the operating system has a fair scheduler. */ + #define EXPLICIT_YIELD 1 + ++/* Whether to use 'volatile' on some variables that communicate information ++ between threads. If set to 0, a lock is used to protect these variables. ++ If set to 1, 'volatile' is used; this is theoretically equivalent but can ++ lead to much slower execution (e.g. 30x slower total run time on a 40-core ++ machine. */ ++#define USE_VOLATILE 0 ++ + /* Whether to print debugging messages. */ + #define ENABLE_DEBUGGING 0 + +@@ -103,6 +110,51 @@ + # define yield() + #endif + ++#if USE_VOLATILE ++struct atomic_int { ++ volatile int value; ++}; ++static void ++init_atomic_int (struct atomic_int *ai) ++{ ++} ++static int ++get_atomic_int_value (struct atomic_int *ai) ++{ ++ return ai->value; ++} ++static void ++set_atomic_int_value (struct atomic_int *ai, int new_value) ++{ ++ ai->value = new_value; ++} ++#else ++struct atomic_int { ++ gl_lock_define (, lock) ++ int value; ++}; ++static void ++init_atomic_int (struct atomic_int *ai) ++{ ++ gl_lock_init (ai->lock); ++} ++static int ++get_atomic_int_value (struct atomic_int *ai) ++{ ++ gl_lock_lock (ai->lock); ++ int ret = ai->value; ++ gl_lock_unlock (ai->lock); ++ return ret; ++} ++static void ++set_atomic_int_value (struct atomic_int *ai, int new_value) ++{ ++ gl_lock_lock (ai->lock); ++ ai->value = new_value; ++ gl_lock_unlock (ai->lock); ++} ++#endif ++ + #define ACCOUNT_COUNT 4 + + static int account[ACCOUNT_COUNT]; +@@ -170,12 +222,12 @@ lock_mutator_thread (void *arg) + return NULL; + } + +-static volatile int lock_checker_done; ++static struct atomic_int lock_checker_done; + + static void * + lock_checker_thread (void *arg) + { +- while (!lock_checker_done) ++ while (get_atomic_int_value (&lock_checker_done) == 0) + { + dbgprintf ("Checker %p before check lock\n", gl_thread_self_pointer ()); + gl_lock_lock (my_lock); +@@ -200,7 +252,8 @@ test_lock (void) + /* Initialization. */ + for (i = 0; i < ACCOUNT_COUNT; i++) + account[i] = 1000; +- lock_checker_done = 0; ++ init_atomic_int (&lock_checker_done); ++ set_atomic_int_value (&lock_checker_done, 0); + + /* Spawn the threads. */ + checkerthread = gl_thread_create (lock_checker_thread, NULL); +@@ -210,7 +263,7 @@ test_lock (void) + /* Wait for the threads to terminate. */ + for (i = 0; i < THREAD_COUNT; i++) + gl_thread_join (threads[i], NULL); +- lock_checker_done = 1; ++ set_atomic_int_value (&lock_checker_done, 1); + gl_thread_join (checkerthread, NULL); + check_accounts (); + } +@@ -254,12 +307,12 @@ rwlock_mutator_thread (void *arg) + return NULL; + } + +-static volatile int rwlock_checker_done; ++static struct atomic_int rwlock_checker_done; + + static void * + rwlock_checker_thread (void *arg) + { +- while (!rwlock_checker_done) ++ while (get_atomic_int_value (&rwlock_checker_done) == 0) + { + dbgprintf ("Checker %p before check rdlock\n", gl_thread_self_pointer ()); + gl_rwlock_rdlock (my_rwlock); +@@ -284,7 +337,8 @@ test_rwlock (void) + /* Initialization. */ + for (i = 0; i < ACCOUNT_COUNT; i++) + account[i] = 1000; +- rwlock_checker_done = 0; ++ init_atomic_int (&rwlock_checker_done); ++ set_atomic_int_value (&rwlock_checker_done, 0); + + /* Spawn the threads. */ + for (i = 0; i < THREAD_COUNT; i++) +@@ -295,7 +349,7 @@ test_rwlock (void) + /* Wait for the threads to terminate. */ + for (i = 0; i < THREAD_COUNT; i++) + gl_thread_join (threads[i], NULL); +- rwlock_checker_done = 1; ++ set_atomic_int_value (&rwlock_checker_done, 1); + for (i = 0; i < THREAD_COUNT; i++) + gl_thread_join (checkerthreads[i], NULL); + check_accounts (); +@@ -356,12 +410,12 @@ reclock_mutator_thread (void *arg) + return NULL; + } + +-static volatile int reclock_checker_done; ++static struct atomic_int reclock_checker_done; + + static void * + reclock_checker_thread (void *arg) + { +- while (!reclock_checker_done) ++ while (get_atomic_int_value (&reclock_checker_done) == 0) + { + dbgprintf ("Checker %p before check lock\n", gl_thread_self_pointer ()); + gl_recursive_lock_lock (my_reclock); +@@ -386,7 +440,8 @@ test_recursive_lock (void) + /* Initialization. */ + for (i = 0; i < ACCOUNT_COUNT; i++) + account[i] = 1000; +- reclock_checker_done = 0; ++ init_atomic_int (&reclock_checker_done); ++ set_atomic_int_value (&reclock_checker_done, 0); + + /* Spawn the threads. */ + checkerthread = gl_thread_create (reclock_checker_thread, NULL); +@@ -396,7 +451,7 @@ test_recursive_lock (void) + /* Wait for the threads to terminate. */ + for (i = 0; i < THREAD_COUNT; i++) + gl_thread_join (threads[i], NULL); +- reclock_checker_done = 1; ++ set_atomic_int_value (&reclock_checker_done, 1); + gl_thread_join (checkerthread, NULL); + check_accounts (); + } diff --git a/gnu/packages/patches/gettext-multi-core.patch b/gnu/packages/patches/gettext-multi-core.patch new file mode 100644 index 000000000..3c7f0c759 --- /dev/null +++ b/gnu/packages/patches/gettext-multi-core.patch @@ -0,0 +1,183 @@ +This patch fixes performance problems on multi-core machines. +See commit 1afbcb06fded2a427b761dd1615b1e48e1e853cc in gettext +from Bruno Haible . + +diff --git a/gettext-runtime/tests/test-lock.c b/gettext-runtime/tests/test-lock.c +index d279d1d60..51cec3d6b 100644 +--- a/gettext-runtime/tests/test-lock.c ++++ b/gettext-runtime/tests/test-lock.c +@@ -1,5 +1,5 @@ + /* Test of locking in multithreaded situations. +- Copyright (C) 2005, 2008-2016 Free Software Foundation, Inc. ++ Copyright (C) 2005, 2008-2017 Free Software Foundation, Inc. + + This program is free software: you can redistribute it and/or modify + it under the terms of the GNU Lesser General Public License as published by +@@ -50,6 +50,13 @@ + Uncomment this to see if the operating system has a fair scheduler. */ + #define EXPLICIT_YIELD 1 + ++/* Whether to use 'volatile' on some variables that communicate information ++ between threads. If set to 0, a lock is used to protect these variables. ++ If set to 1, 'volatile' is used; this is theoretically equivalent but can ++ lead to much slower execution (e.g. 30x slower total run time on a 40-core ++ machine. */ ++#define USE_VOLATILE 0 ++ + /* Whether to print debugging messages. */ + #define ENABLE_DEBUGGING 0 + +@@ -214,6 +221,51 @@ static inline void * gl_thread_self_pointer (void) + # define yield() + #endif + ++#if USE_VOLATILE ++struct atomic_int { ++ volatile int value; ++}; ++static void ++init_atomic_int (struct atomic_int *ai) ++{ ++} ++static int ++get_atomic_int_value (struct atomic_int *ai) ++{ ++ return ai->value; ++} ++static void ++set_atomic_int_value (struct atomic_int *ai, int new_value) ++{ ++ ai->value = new_value; ++} ++#else ++struct atomic_int { ++ gl_lock_define (, lock) ++ int value; ++}; ++static void ++init_atomic_int (struct atomic_int *ai) ++{ ++ gl_lock_init (ai->lock); ++} ++static int ++get_atomic_int_value (struct atomic_int *ai) ++{ ++ gl_lock_lock (ai->lock); ++ int ret = ai->value; ++ gl_lock_unlock (ai->lock); ++ return ret; ++} ++static void ++set_atomic_int_value (struct atomic_int *ai, int new_value) ++{ ++ gl_lock_lock (ai->lock); ++ ai->value = new_value; ++ gl_lock_unlock (ai->lock); ++} ++#endif ++ + #define ACCOUNT_COUNT 4 + + static int account[ACCOUNT_COUNT]; +@@ -281,12 +333,12 @@ lock_mutator_thread (void *arg) + return NULL; + } + +-static volatile int lock_checker_done; ++static struct atomic_int lock_checker_done; + + static void * + lock_checker_thread (void *arg) + { +- while (!lock_checker_done) ++ while (get_atomic_int_value (&lock_checker_done) == 0) + { + dbgprintf ("Checker %p before check lock\n", gl_thread_self_pointer ()); + gl_lock_lock (my_lock); +@@ -311,7 +363,8 @@ test_lock (void) + /* Initialization. */ + for (i = 0; i < ACCOUNT_COUNT; i++) + account[i] = 1000; +- lock_checker_done = 0; ++ init_atomic_int (&lock_checker_done); ++ set_atomic_int_value (&lock_checker_done, 0); + + /* Spawn the threads. */ + checkerthread = gl_thread_create (lock_checker_thread, NULL); +@@ -321,7 +374,7 @@ test_lock (void) + /* Wait for the threads to terminate. */ + for (i = 0; i < THREAD_COUNT; i++) + gl_thread_join (threads[i], NULL); +- lock_checker_done = 1; ++ set_atomic_int_value (&lock_checker_done, 1); + gl_thread_join (checkerthread, NULL); + check_accounts (); + } +@@ -365,12 +418,12 @@ rwlock_mutator_thread (void *arg) + return NULL; + } + +-static volatile int rwlock_checker_done; ++static struct atomic_int rwlock_checker_done; + + static void * + rwlock_checker_thread (void *arg) + { +- while (!rwlock_checker_done) ++ while (get_atomic_int_value (&rwlock_checker_done) == 0) + { + dbgprintf ("Checker %p before check rdlock\n", gl_thread_self_pointer ()); + gl_rwlock_rdlock (my_rwlock); +@@ -395,7 +448,8 @@ test_rwlock (void) + /* Initialization. */ + for (i = 0; i < ACCOUNT_COUNT; i++) + account[i] = 1000; +- rwlock_checker_done = 0; ++ init_atomic_int (&rwlock_checker_done); ++ set_atomic_int_value (&rwlock_checker_done, 0); + + /* Spawn the threads. */ + for (i = 0; i < THREAD_COUNT; i++) +@@ -406,7 +460,7 @@ test_rwlock (void) + /* Wait for the threads to terminate. */ + for (i = 0; i < THREAD_COUNT; i++) + gl_thread_join (threads[i], NULL); +- rwlock_checker_done = 1; ++ set_atomic_int_value (&rwlock_checker_done, 1); + for (i = 0; i < THREAD_COUNT; i++) + gl_thread_join (checkerthreads[i], NULL); + check_accounts (); +@@ -467,12 +521,12 @@ reclock_mutator_thread (void *arg) + return NULL; + } + +-static volatile int reclock_checker_done; ++static struct atomic_int reclock_checker_done; + + static void * + reclock_checker_thread (void *arg) + { +- while (!reclock_checker_done) ++ while (get_atomic_int_value (&reclock_checker_done) == 0) + { + dbgprintf ("Checker %p before check lock\n", gl_thread_self_pointer ()); + gl_recursive_lock_lock (my_reclock); +@@ -497,7 +551,8 @@ test_recursive_lock (void) + /* Initialization. */ + for (i = 0; i < ACCOUNT_COUNT; i++) + account[i] = 1000; +- reclock_checker_done = 0; ++ init_atomic_int (&reclock_checker_done); ++ set_atomic_int_value (&reclock_checker_done, 0); + + /* Spawn the threads. */ + checkerthread = gl_thread_create (reclock_checker_thread, NULL); +@@ -507,7 +562,7 @@ test_recursive_lock (void) + /* Wait for the threads to terminate. */ + for (i = 0; i < THREAD_COUNT; i++) + gl_thread_join (threads[i], NULL); +- reclock_checker_done = 1; ++ set_atomic_int_value (&reclock_checker_done, 1); + gl_thread_join (checkerthread, NULL); + check_accounts (); + } -- 2.11.1 --=-=-=--