Re: no pthread_spinlock_t on Mac OS 10.6.4

[Gary V. Vaughan]

Hi Paul,

Thanks for looking at this.

On 20 Sep 2010, at 13:21, Paul Eggert wrote:
> On 09/19/2010 07:43 PM, Gary V. Vaughan wrote:
>> my system headers have no pthread_spinlock_t definition, but
>> gnulib sees /usr/include/pthread.h and uses that instead of generating it's 
>> own,
>> ...
>> I don't know enough about pthreads to tell whether gnulib should paper over
>> the lack of spinlocks on Mac OS, or whether coreutils should be more careful
>> about using spinlocks without having tested for them first...
> 
> If MacOS doesn't have spinlocks, then from coreutils' point of view MacOS
> doesn't have proper thread support.  The simplest fix is for coreutils
> to use the substitute pthread.h on MacOS.  Does the following patch
> work for you?

It fixes the original spinlocks error I reported, but then the build falls over 
on lib/glthread/lock.c:

In file included from ../../lib/glthread/lock.h:93,
 from ../../lib/glthread/lock.c:26:
./pthread.h:184: error: expected ')' before '*' token
./pthread.h:191: error: expected ')' before '*' token
./pthread.h:198: error: expected ')' before '*' token
./pthread.h:206: error: expected ')' before '*' token
../../lib/glthread/lock.c: In function 
'glthread_recursive_lock_init_multithreaded':
../../lib/glthread/lock.c:321: warning: implicit declaration of function 
'pthread_mutexattr_init'
../../lib/glthread/lock.c:324: warning: implicit declaration of function 
'pthread_mutexattr_settype'
../../lib/glthread/lock.c:327: warning: implicit declaration of function 
'pthread_mutexattr_destroy'

where lines 184, 191, 198 and 206 of lib/pthread.h each contain a use of 
pthread_spinlock_t.

lib/pthread.h does have a typedef for pthread_spinlock_t, but it is predicated 
on not having HAVE_PTHREAD_T defined (which I do):

  ; sed -n /HAVE_PTHREAD_T/,/endif/p lib/pthread.h 
  #ifndef HAVE_PTHREAD_T
   typedef int pthread_t;
   typedef int pthread_attr_t;
   typedef int pthread_barrier_t;
   typedef int pthread_barrierattr_t;
   typedef int pthread_cond_t;
   typedef int pthread_condattr_t;
   typedef int pthread_key_t;
   typedef int pthread_mutex_t;
   typedef int pthread_mutexattr_t;
   typedef int pthread_once_t;
   typedef int pthread_rwlock_t;
   typedef int pthread_rwlockattr_t;
   typedef int pthread_spinlock_t;
  #endif
  ; grep HAVE_PTHREAD_T lib/config.h
  #define HAVE_PTHREAD_T 1

> diff --git a/m4/pthread.m4 b/m4/pthread.m4
> index 69866cb..121d84d 100644
> --- a/m4/pthread.m4
> +++ b/m4/pthread.m4
> @@ -6,19 +6,22 @@ dnl with or without modifications, as long as this notice 
> is preserved.
> 
> AC_DEFUN([gl_PTHREAD_CHECK],
>   [AC_CHECK_HEADERS_ONCE([pthread.h])
> +   AC_CHECK_TYPES([pthread_t])
> 
>LIB_PTHREAD=
> -   PTHREAD_H=
> -   if test "$ac_cv_header_pthread_h" = yes; then
> - gl_saved_libs=$LIBS
> - AC_SEARCH_LIBS([pthread_create], [pthread],
> -   [if test "$ac_cv_search_pthread_create" != "none required"; then
> -  LIB_PTHREAD="$ac_cv_search_pthread_create"
> -fi])
> - LIBS="$gl_saved_libs"
> -   else
> - AC_CHECK_TYPES([pthread_t])
> - PTHREAD_H='pthread.h'
> +   PTHREAD_H='pthread.h'
> +   if test "$ac_cv_header_pthread_h" = yes &&
> +  test "$ac_cv_type_pthread_t" = yes; then
> + AC_CHECK_TYPE([pthread_spinlock_t])
> + if test "$ac_cv_type_pthread_spinlock_t" = yes; then
> +   PTHREAD_H=
> +   gl_saved_libs=$LIBS
> +   AC_SEARCH_LIBS([pthread_create], [pthread],
> + [if test "$ac_cv_search_pthread_create" != "none required"; then
> +LIB_PTHREAD="$ac_cv_search_pthread_create"
> +  fi])
> +   LIBS="$gl_saved_libs"
> + fi
>fi
> 
>AC_SUBST([LIB_PTHREAD])

Cheers,
-- 
Gary V. Vaughan (g...@gnu.org)

PGP.sig
Description: This is a digitally signed message part

Re: build-aux confusion

[Reuben Thomas]

On 20 September 2010 01:27, Karl Berry  wrote:
>    Honestly, what is the problem with gnulib-tool? Why do you find it
>    hard to understand?
>
> Personally, I think g-t is about as well written as anything of its size
> and purpose could be, but 6000 lines of anything is not prima facie
> obvious and easy to understand, regardless of language.

I'm hoping here that Bruno was as tongue-in-cheek as I was...reading
6,000 lines of anything is indeed not very easy. Reading 6,000 lines
of shell is rather like trying to take dictation from a crowd of
people whispering in your ears.

>    not easily possible to determine the default value for a variable?!
>
> Ruben, I commend sh -vx to you :).

Thanks! I'll try to remember for when it's actually life-or-death.

-- 
http://rrt.sc3d.org

Re: no pthread_spinlock_t on Mac OS 10.6.4

[Bruno Haible]

Hi Paul,

> If MacOS doesn't have spinlocks, then from coreutils' point of view MacOS
> doesn't have proper thread support.

Usually in applications mutexes are used, not spinlocks. The comments in sort.c
say:

  "Note spin locks were seen to perform better than mutexes
   as long as the number of threads is limited to the
   number of processors."

In other words, spinlocks were chosen instead of mutexes only for performance
reasons.

So, instead of turning off the entire thread-based parallelization on MacOS X,
why not use mutexes instead of spinlocks on this platform?

Bruno

Re: bug#7073: no pthread_spinlock_t on Mac OS 10.6.4

[Pádraig Brady]

On 20/09/10 07:21, Paul Eggert wrote:
> On 09/19/2010 07:43 PM, Gary V. Vaughan wrote:
>>  my system headers have no pthread_spinlock_t definition, but
>> gnulib sees /usr/include/pthread.h and uses that instead of generating it's 
>> own,
>> ...
>> I don't know enough about pthreads to tell whether gnulib should paper over
>> the lack of spinlocks on Mac OS, or whether coreutils should be more careful
>> about using spinlocks without having tested for them first...
> 
> If MacOS doesn't have spinlocks, then from coreutils' point of view MacOS
> doesn't have proper thread support.  The simplest fix is for coreutils
> to use the substitute pthread.h on MacOS.  Does the following patch
> work for you?

On a related note, I've been meaning to check
if mutexes in coreutils/sort are more stable
and/or fair to the system than spinlocks.
If we don't end up moving to mutexes everywhere,
perhaps we should do it for Mac OS always?

Also on the same point of more appropriate use
of system resources, perhaps we should cap the
number of cores used to perhaps 8 given results like:
http://debbugs.gnu.org/cgi/bugreport.cgi?msg=49;filename=sort-amd-24way.png;att=1;bug=6600
We would allow increasing up to the number of cores
available on the system by user specified values
(which currently only support decreasing from #cores).

cheers,
Pádraig.

Re: new module 'regex-quote'

[Eric Blake]

On 09/18/2010 04:05 PM, Bruno Haible wrote:

/* regex_quote converts a literal string to a regular expression that will
look for this literal string.
cflags can be 0 or REG_EXTENDED.
If it is 0, the result is a Basic Regular Expression (BRE)

.
If it is REG_EXTENDED, the result is an Extended Regular Expression (ERE)

.
The result is not anchored;  if you want it to match only complete lines,
you need to add "^" at the beginning of the result and "$" at the end of the
result.
  */

/* Returns the number of bytes needed for the quoted string.  */
extern size_t regex_quote_length (const char *string, int cflags);


Since we're already passing a flag, can we add another flag that adds 
the ^ and $ anchors?  It's more convenient to do just one malloc (via 
the regex_quote) that also adds the anchors, than it is to have to 
manually call regex_quote_length, increment the result, malloc the 
space, insert the anchors, then call regex_quote_copy.


--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

Re: new module 'regex-quote'

[Reuben Thomas]

On 18 September 2010 23:53, Reuben Thomas  wrote:
> Great, thanks, though at 3 new functions and about 50 lines of code it
> does illustrate why I prefer RE_PLAIN.
>
> Nonetheless, if you put this in gnulib I'd probably use it rather than
> RE_PLAIN, just because it's there.

On second thoughts, --local-dir is easy enough to use that if the
regex code doesn't change much in future, I'll probably just maintain
RE_PLAIN. Congratulations to whoever dreamed up the simple override
mechanism, plus ten bonus points for being able to use diffs as well
as plain files.

-- 
http://rrt.sc3d.org

Re: A fix for pmccabe2html

[Eric Blake]

On 09/19/2010 04:54 PM, Reuben Thomas wrote:

After investigation, I think I found the bug: cut_dir should not be
hardwired to "/../", but rather set to "${top_srcdir}/". Otherwise, it
does not work for a typical in-tree build: cut_dir is not found in the
path, index returns 0, and an arbitrary few characters are cut off the
front of each file's path. The fix is therefore to remove the
initialization of cut_dir from pmccabe2html (1 line) and add "-v
cut_dir=${top_srcdir}/" to the Makefile.am code.


Rather than describing the fix, could you post this as a patch?

--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

Re: module libposix

[Bruce Korb]

Hi Bruno,

On Sat, Sep 18, 2010 at 5:21 PM, Bruno Haible  wrote:
>> >   ./build-aux/git-version-gen /dev/null | sed -e 's/-dirty/-modified/'
>>
>> "UNKNOWN-modified"?  I don't know how you'd want to see that worked
>> into a version number.
>
> Hmm, for me that command prints
>  0.0.4280-882da
> Is your git version older than 1.6 ?

I took a quick look.  The problem is that "git-version-gen" knows
that it must be running in the gnulib root directory.  If it is not,
it just quietly inserts "UNKNOWN" into the output.  This script should
add this code early on:

cd_to_git_root() {
   test -d .git && return 0
   d=`dirname $0`/..
   test -d $d/.git && cd $d && return 0
   d=`git root`
   test -d $d/.git && cd $d && return 0
  die "cannot determine git version outside of repo"
}

cd_to_git_root

Re: module libposix

[Eric Blake]

On 09/20/2010 09:13 AM, Bruce Korb wrote:

I took a quick look.  The problem is that "git-version-gen" knows
that it must be running in the gnulib root directory.  If it is not,
it just quietly inserts "UNKNOWN" into the output.  This script should
add this code early on:

cd_to_git_root() {
test -d .git&&  return 0
d=`dirname $0`/..
test -d $d/.git&&  cd $d&&  return 0
d=`git root`
test -d $d/.git&&  cd $d&&  return 0
   die "cannot determine git version outside of repo"
}


Except that this won't help projects that use git-version-gen inside 
configure.ac to generate the package version number, where the script 
MUST succeed (even if by printing UNKNOWN) so that you can autoreconf 
the project even when outside of a git repository.  Furthermore, testing 
for the existence of .git is not the kosher way of determining whether 
you are running from within a git repository, and there is no such 
command as 'git root'.


--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

Re: module libposix

[Bruce Korb]

Hi,

On Mon, Sep 20, 2010 at 8:33 AM, Eric Blake  wrote:
> On 09/20/2010 09:13 AM, Bruce Korb wrote:
>>
>> I took a quick look.  The problem is that "git-version-gen" knows
>> that it must be running in the gnulib root directory.  If it is not,
>> it just quietly inserts "UNKNOWN" into the output.  This script should
>> add this code early on:
>>
>> cd_to_git_root() {
>>    test -d .git&&  return 0
>>    d=`dirname $0`/..
>>    test -d $d/.git&&  cd $d&&  return 0
>>    d=`git root`
>>    test -d $d/.git&&  cd $d&&  return 0
>>   die "cannot determine git version outside of repo"
>> }
>
> Except that this won't help projects that use git-version-gen inside
> configure.ac to generate the package version number, where the script MUST
> succeed (even if by printing UNKNOWN) so that you can autoreconf the project
> even when outside of a git repository.  Furthermore, testing for the
> existence of .git is not the kosher way

Perhaps not, but it currently does that test to determine if
"UNKNOWN" should be emitted.  I was merely copying that logic.
In my case, I want to choke over UNKNOWN.

> of determining whether you are
> running from within a git repository, and there is no such command
> as 'git root'.

Sorry.  BitKeeper had it and the method for accomplishing it was
too complicated, so I added an alias and forgot I'd done so:
[alias]
root = !pwd -P

There is an approved way, but it is really obtuse:
  $(\cd ./$(git rev-parse --show-cdup) && pwd -P)
Far from the first thing that comes to mind.

Re: new module 'regex-quote'

[Bruno Haible]

Hi Eric,

> > /* regex_quote converts a literal string to a regular expression that will
> > look for this literal string.
> > cflags can be 0 or REG_EXTENDED.
> > If it is 0, the result is a Basic Regular Expression (BRE)
> > 
> > .
> > If it is REG_EXTENDED, the result is an Extended Regular Expression 
> > (ERE)
> > 
> > .
> > The result is not anchored;  if you want it to match only complete 
> > lines,
> > you need to add "^" at the beginning of the result and "$" at the end 
> > of the
> > result.
> 
> Since we're already passing a flag, can we add another flag that adds 
> the ^ and $ anchors?  It's more convenient to do just one malloc (via 
> the regex_quote) that also adds the anchors, than it is to have to 
> manually call regex_quote_length, increment the result, malloc the 
> space, insert the anchors, then call regex_quote_copy.

It's an interesting suggestion. But frankly, the benefit of such an option is
small: It's straightforward, even in C, to prepend a "^" and append a "$" to 
a string. In the way you describe, or through
   regex = xasprintf ("^%s$", regex);
And adding this flag would mean that the cflags of regex_quote no longer are
a subset of the flags passed to regcomp.

So, for the moment (until there's more heavy demand for such a flag), I prefer
to address this issue only through the comments above.

Bruno

Re: new module 'regex-quote'

[Eric Blake]

On 09/20/2010 10:21 AM, Bruno Haible wrote:

Hi Eric,


/* regex_quote converts a literal string to a regular expression that will
 look for this literal string.
 cflags can be 0 or REG_EXTENDED.
 If it is 0, the result is a Basic Regular Expression (BRE)
 
.
 If it is REG_EXTENDED, the result is an Extended Regular Expression (ERE)
 
.
 The result is not anchored;  if you want it to match only complete lines,
 you need to add "^" at the beginning of the result and "$" at the end of 
the
 result.


Since we're already passing a flag, can we add another flag that adds
the ^ and $ anchors?  It's more convenient to do just one malloc (via
the regex_quote) that also adds the anchors, than it is to have to
manually call regex_quote_length, increment the result, malloc the
space, insert the anchors, then call regex_quote_copy.


It's an interesting suggestion. But frankly, the benefit of such an option is
small: It's straightforward, even in C, to prepend a "^" and append a "$" to
a string. In the way you describe, or through
regex = xasprintf ("^%s$", regex);
And adding this flag would mean that the cflags of regex_quote no longer are
a subset of the flags passed to regcomp.

So, for the moment (until there's more heavy demand for such a flag), I prefer
to address this issue only through the comments above.


Or, rather than adding a new flag big, what about providing 
regex_quote_anchored as a wrapper that provides the anchors, so that the 
end user can choose between the one-liners of regex_quote or 
regex_quote_anchored?


--
Eric Blake   ebl...@redhat.com+1-801-349-2682
Libvirt virtualization library http://libvirt.org

Re: no pthread_spinlock_t on Mac OS 10.6.4

[Paul Eggert]

On 09/20/10 00:14, Gary V. Vaughan wrote:

> lib/pthread.h does have a typedef for pthread_spinlock_t, but it is 
> predicated on not having HAVE_PTHREAD_T defined (which I do):

Ah, OK.  Could you try the following patch instead?  If you have a
similar problem with (say) pthread_rwlockattr_t, you can treat it just
like pthread_spinlock_t in lib/pthread.in.h and m4/pthread.m4; please
just let us know which types those are.  I'd
rather not add compile-time checks for each dinky little pthread type
unless it's known to be needed, as 'configure' takes too long already.

diff --git a/lib/pthread.in.h b/lib/pthread.in.h
index 4dad22a..84cf913 100644
--- a/lib/pthread.in.h
+++ b/lib/pthread.in.h
@@ -40,6 +40,8 @@
  typedef int pthread_once_t;
  typedef int pthread_rwlock_t;
  typedef int pthread_rwlockattr_t;
+#endif
+#ifndef HAVE_PTHREAD_SPINLOCK_T
  typedef int pthread_spinlock_t;
 #endif
 
diff --git a/m4/pthread.m4 b/m4/pthread.m4
index 69866cb..a2781eb 100644
--- a/m4/pthread.m4
+++ b/m4/pthread.m4
@@ -6,19 +6,21 @@ dnl with or without modifications, as long as this notice is 
preserved.
 
 AC_DEFUN([gl_PTHREAD_CHECK],
   [AC_CHECK_HEADERS_ONCE([pthread.h])
+   gl_keep_pthread_h=$ac_cv_header_pthread_h
+   AC_CHECK_TYPES([pthread_t, pthread_spinlock_t],
+ [],
+ [gl_keep_pthread_h=no])
 
+   PTHREAD_H='pthread.h'
LIB_PTHREAD=
-   PTHREAD_H=
-   if test "$ac_cv_header_pthread_h" = yes; then
+   if test "$gl_keep_pthread_h" = yes; then
+ PTHREAD_H=
  gl_saved_libs=$LIBS
  AC_SEARCH_LIBS([pthread_create], [pthread],
[if test "$ac_cv_search_pthread_create" != "none required"; then
   LIB_PTHREAD="$ac_cv_search_pthread_create"
 fi])
  LIBS="$gl_saved_libs"
-   else
- AC_CHECK_TYPES([pthread_t])
- PTHREAD_H='pthread.h'
fi
 
AC_SUBST([LIB_PTHREAD])

Re: no pthread_spinlock_t on Mac OS 10.6.4

[Paul Eggert]

On 09/20/10 02:24, Bruno Haible wrote:
> why not use mutexes instead of spinlocks on this platform?

That might be a good thing to try, though it'd take more work.

Chen, what do you think?  Here's the email that started this discussion:
http://lists.gnu.org/archive/html/bug-coreutils/2010-09/msg00065.html

Re: bug#7073: no pthread_spinlock_t on Mac OS 10.6.4

[Paul Eggert]

On 09/20/10 02:33, Pádraig Brady wrote:
> On a related note, I've been meaning to check
> if mutexes in coreutils/sort are more stable
> and/or fair to the system than spinlocks.

They ought to be fairer, though I'd expect there to be a significant
performance price in some cases.  I'm not sure what you mean
by "stable" (surely this has nothing to do with stable sorting :-).

It's a bit tricky, as some users prefer performance, others fairness,
and the default can't satisfy everybody.

> perhaps we should cap the
> number of cores used to perhaps 8 given results like:
> http://debbugs.gnu.org/cgi/bugreport.cgi?msg=49;filename=sort-amd-24way.png;att=1;bug=6600
> We would allow increasing up to the number of cores
> available on the system by user specified values
> (which currently only support decreasing from #cores).

That result depends on the particular benchmark and platform,
of course.  Still, it might make sense to default to at most 8
for now, as you suggest; the documentation should say that that
default might change of course.

Another thought: the syntax for --parallel might be generalized
to allow for fancier heuristics, e.g.:

--parallel=50%

would mean to use at most half the processors.  This particular
syntax would be similar to how --buffer-size acts.

Re: new module 'regex-quote'

[Bruno Haible]

Eric,

> rather than adding a new flag bit, what about providing 
> regex_quote_anchored as a wrapper that provides the anchors, so that the 
> end user can choose between the one-liners of regex_quote or 
> regex_quote_anchored?

It's the same argumentation: It's already easy enough to do this in C, with
1 to 5 straightforward lines of code.

When you provide an API, where to stop? When an API provides two similar
functionalities, where one can be coded using the other and 1-5 extra lines of
code, one of them is redundant.
- Once we have memcpy, is it worth having strcpy and stpcpy? Normally no. But
  there are so many uses of memcpy that adding strcpy and stpcpy was worthwhile.
- Once we have strpbrk, is it worth having strcspn? I would say no. strcspn is
  only there because it predates strpbrk.
- Once we have regex_quote, regex_quote_anchored is redundant.

The drawback of a large API is that it's harder to keep in memory.

Bruno

Re: new module 'regex-quote'

[Bruce Korb]

> The drawback of a large API is that it's harder to keep in memory.

"keep in mind".  "memory" as in "computer memory" is too cheap to
be any consideration at all. :)

Re: bug#7073: no pthread_spinlock_t on Mac OS 10.6.4

[Chen Guo]

Hi all,

First regarding mutexes on Macs, I suppose a performance hit with mutexes beats
no performance at all with missing spinlocks. And regarding "take more work," I
believe spinlocks and mutexes were basically interchangeable in terms
of functionality
in our sort algorithm; the work probably will be little more than a
few #ifdefs and a
s/pthread_mutex_t/pthread_spinlock_t/

How difficult would it be to implement a basic spinlock in gnulib,
though? Perhaps we
can implement that and fall back to it on MacOS X?


>> On a related note, I've been meaning to check
>> if mutexes in coreutils/sort are more stable
>> and/or fair to the system than spinlocks.
>
> They ought to be fairer, though I'd expect there to be a significant
> performance price in some cases.  I'm not sure what you mean
> by "stable" (surely this has nothing to do with stable sorting :-).
>

An example of the stability issues is be the hyperthreaded processor issue.
For example, when I was sorting on an i7 (4 cores, 8 logical), sort time with
threads > 4 had very high variance with spinlocks, but not with mutexes.

I do believe this particular issue is handled; I remember capping threads
to the number of physical cores on the system. Of course, there might be
other nasties that we just don't know about yet with spinlocks.


>> perhaps we should cap the
>> number of cores used to perhaps 8 given results like:
>> http://debbugs.gnu.org/cgi/bugreport.cgi?msg=49;filename=sort-amd-24way.png;att=1;bug=6600
>> We would allow increasing up to the number of cores
>> available on the system by user specified values
>> (which currently only support decreasing from #cores).
>
> That result depends on the particular benchmark and platform,
> of course.  Still, it might make sense to default to at most 8
> for now, as you suggest; the documentation should say that that
> default might change of course.
>

This is a good idea... I'm not sure how feasible this would be, but perhaps
we can benchmark some common server systems or some performance
outliers and define a custom max for these.

Re: new module 'regex-quote'

[Reuben Thomas]

On 20 September 2010 19:21, Bruno Haible  wrote:
> The drawback of a large API is that it's harder to keep in memory.

Heh. My point precisely: 3 functions and 50 lines versus 1 flag and 5
lines (RE_PLAIN) to solve the same problem (although my solution does
not help with the anchored variant, but neither does it make it
harder).

-- 
http://rrt.sc3d.org

Re: A fix for pmccabe2html

[Reuben Thomas]

On 20 September 2010 15:49, Eric Blake  wrote:
> Rather than describing the fix, could you post this as a patch?

Sorry; you've made me realise that my habit of not posting patches for
code I'm not sure about is silly for several reasons.

Patch attached. I've added an Emacs mode-setting line to the top of
the file so that Emacs gets the right editing mode; is there some
reason not to have a hash-bang line?

-- 
http://rrt.sc3d.org


0001-Set-cut_dir-properly-and-add-mode-line-for-Emacs.patch
Description: Binary data

Another minor patch to pmccabe2html

[Reuben Thomas]

Attached, to make the whitespace in the Makefile.am example more
copy-and-paste friendly.

By the way, is there some reason to keep this file and pmccabe.css in
gnulib rather than push it upstream to pmccabe, and make the gnulib
module just make the test and suggest an appropriate Makefile.am patch
with settings for GNU projects?

-- 
http://rrt.sc3d.org


0002-Make-Makefile.am-example-code-more-cut-and-paste-fri.patch
Description: Binary data

[PATCH] pthread: add pthread_spin_destroy

[Paul Eggert]

* lib/pthread.in.h (pthread_spin_destroy): New function.
---
 ChangeLog|5 +
 lib/pthread.in.h |7 +++
 2 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 307a58a..ec48b91 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,8 @@
+2010-09-20  Paul Eggert  
+
+   pthread: add pthread_spin_destroy
+   * lib/pthread.in.h (pthread_spin_destroy): New function.
+
 2010-09-19  Bruno Haible  
 
gnulib-tool: Fix --help output.
diff --git a/lib/pthread.in.h b/lib/pthread.in.h
index 4dad22a..0fdf9c3 100644
--- a/lib/pthread.in.h
+++ b/lib/pthread.in.h
@@ -186,6 +186,13 @@ pthread_spin_init (pthread_spinlock_t *lock, int pshared)
 }
 
 static inline int
+pthread_spin_destroy (pthread_spinlock_t *lock)
+{
+  /* LOCK is never seriously used.  */
+  return 0;
+}
+
+static inline int
 pthread_spin_lock (pthread_spinlock_t *lock)
 {
   /* Only one thread, so it always gets the lock.  */
-- 
1.7.2

Re: bug#7073: no pthread_spinlock_t on Mac OS 10.6.4

[Paul Eggert]

On 09/20/10 14:38, Chen Guo wrote:

> How difficult would it be to implement a basic spinlock in gnulib, though?

Portably?  I'd think it'd be quite a pain, as it would require
figuring out this platform's atomic instructions, dealing with memory
barriers, and the like.

> I suppose a performance hit with mutexes beats no performance at all
> with missing spinlocks.

Yes.

> And regarding "take more work," I believe spinlocks and mutexes were
> basically interchangeable in terms of functionality in our sort
> algorithm; the work probably will be little more than a few #ifdefs
> and a s/pthread_mutex_t/pthread_spinlock_t/

Is that a reasonably-valid replacement in general, for code that uses
spin locks?  If so, we should implement this inside gnulib's pthread
module.  If not, it needs to be done inside coreutils, or perhaps
as a separate gnulib module.

Re: new module 'regex-quote'

[Bruno Haible]

Reuben,

> Heh. My point precisely: 3 functions and 50 lines versus 1 flag and 5
> lines (RE_PLAIN) to solve the same problem

I agree that if we had the opportunity to invent regex APIs from scratch
now, all 4 syntaxes (literals, wildcards, basic regular expression, extended
regular expression) would be worth supporting equally.

But the fact is that POSIX standardizes the regex API, and therefore there
is a border between "in glibc" and "outside glibc". Functionality in glibc
is available at no cost; functionality outside glibc requires additional
link options and increased startup time or a 50KB bigger executable.

Bruno

Re: no pthread_spinlock_t on Mac OS 10.6.4

[Gary V. Vaughan]

Hi Paul,

On 21 Sep 2010, at 00:30, Paul Eggert wrote:
> On 09/20/10 00:14, Gary V. Vaughan wrote:
> 
>> lib/pthread.h does have a typedef for pthread_spinlock_t, but it is 
>> predicated on not having HAVE_PTHREAD_T defined (which I do):
> 
> Ah, OK.  Could you try the following patch instead?

The patch you attached allows compilation to complete on my mac, and produces a 
working (according to my very cursory testing!) sort binary.

I notice the following warnings though:

../../lib/glthread/lock.c: In function 
'glthread_recursive_lock_init_multithreaded':
../../lib/glthread/lock.c:319: warning: implicit declaration of function 
'pthread_mutexattr_init'
../../lib/glthread/lock.c:322: warning: implicit declaration of function 
'pthread_mutexattr_settype'
../../lib/glthread/lock.c:325: warning: implicit declaration of function 
'pthread_mutexattr_destroy'

Which is odd, because /usr/include/pthread.h contains (unguarded) prototypes 
for all those functions - so I assume the system pthread.h is either being 
mangled, or entirely skipped when glthread/lock.c includes the gnulib/pthread.h?

* time passes *

Hmm... looking at gnulib/pthread.h, am I right in assuming that threads are now 
effectively disabled by gnulib on the mac in favour of a selection of stub 
functions?  In which case, I suppose those warnings indicate that 
glthread/lock.c may attempt to call unstubbed functions along some codepath and 
will then crash.

> diff --git a/lib/pthread.in.h b/lib/pthread.in.h
> index 4dad22a..84cf913 100644
> --- a/lib/pthread.in.h
> +++ b/lib/pthread.in.h
> @@ -40,6 +40,8 @@
>  typedef int pthread_once_t;
>  typedef int pthread_rwlock_t;
>  typedef int pthread_rwlockattr_t;
> +#endif
> +#ifndef HAVE_PTHREAD_SPINLOCK_T
>  typedef int pthread_spinlock_t;
> #endif
> 
> diff --git a/m4/pthread.m4 b/m4/pthread.m4
> index 69866cb..a2781eb 100644
> --- a/m4/pthread.m4
> +++ b/m4/pthread.m4
> @@ -6,19 +6,21 @@ dnl with or without modifications, as long as this notice 
> is preserved.
> 
> AC_DEFUN([gl_PTHREAD_CHECK],
>   [AC_CHECK_HEADERS_ONCE([pthread.h])
> +   gl_keep_pthread_h=$ac_cv_header_pthread_h
> +   AC_CHECK_TYPES([pthread_t, pthread_spinlock_t],
> + [],
> + [gl_keep_pthread_h=no])
> 
> +   PTHREAD_H='pthread.h'
>LIB_PTHREAD=
> -   PTHREAD_H=
> -   if test "$ac_cv_header_pthread_h" = yes; then
> +   if test "$gl_keep_pthread_h" = yes; then
> + PTHREAD_H=
>  gl_saved_libs=$LIBS
>  AC_SEARCH_LIBS([pthread_create], [pthread],
>[if test "$ac_cv_search_pthread_create" != "none required"; then
>   LIB_PTHREAD="$ac_cv_search_pthread_create"
> fi])
>  LIBS="$gl_saved_libs"
> -   else
> - AC_CHECK_TYPES([pthread_t])
> - PTHREAD_H='pthread.h'
>fi
> 
>AC_SUBST([LIB_PTHREAD])

Cheers,
-- 
Gary V. Vaughan (g...@gnu.org)



PGP.sig
Description: This is a digitally signed message part