svn commit: r215071 - in user/davidxu/libthr: include lib/libc lib/libc/gen lib/libc/stdio lib/libthr lib/libthr/thread

[Jilles Tjoelker]

On Fri, Nov 12, 2010 at 09:29:01PM +0800, David Xu wrote:
> Jilles Tjoelker wrote:
> > Apart from supporting process-shared, this also helps avoid the "array
> > of synchronization objects" anti-pattern (false sharing). It is not so
> > bad for the old struct pthread_mutex which is 64 bytes on i386, but in
> > other cases one cache line may contain parts of multiple unrelated
> > synchronization objects.

> Don't know what did you mean, did you think that sizeof  struct 
> pthread_mutex
> smaller than a cache line is a bad thing ?

It is a bad thing if the rest of the cache line is not used for a
related purpose.

Code like:
  lock1 = malloc(sizeof(struct pthread_mutex));
  lock2 = malloc(sizeof(struct pthread_mutex));
is likely to place the two locks in the same cache line, which may
defeat much of the advantage of having two locks instead of one.

With the new pthread_mutex_t, applications can do
  struct foo {
    int protectedfield1;
    int protectedfield2;
    pthread_mutex_t lock;
  };
and the cache line fetch for the lock will bring in the protected fields
as well. Applications are responsible for making the struct large enough
to avoid false sharing.

> > In this regard, it would be better for stdio to allocate
> >   struct { FILE file; pthread_mutex_t lock; }
> > rather than separate FILEs and locks.

> This breaks ABI. :-)

Yes, but you can keep the pthread_mutex_t *_fl_mutex and keep accessing
the mutex using that. My concern is only about the way things are laid
out in memory, your approach is otherwise good.

I'm including a lightly tested patch with what I'm thinking of. Of
course, stdio is not a high-performance I/O facility, but it is used
quite a bit and doing it right here may serve as a good example.

> > Like the sem_t change, this changes the ABI in a way symver can only
> > partially deal with (think plugins with pthread_mutex_t in struct in
> > public header file). However, I think the change should be made
> > regardless.

> Yes, we found that some GNU libraries have to bump version.

-- 
Jilles Tjoelker
-------------- next part --------------
A non-text attachment was scrubbed...
Name: stdio-mutex-layout.patch
Type: text/x-diff
Size: 6411 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/svn-src-user/attachments/20101113/e81dc22d/stdio-mutex-layout.bin