Opened 9 years ago

Closed 3 years ago

#5694 closed enhancement (fixed)

Implement pthread barriers

Reported by: bonefish Owned by: Barrett
Priority: normal Milestone: Unscheduled
Component: System/POSIX Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

Implement pthread barrier functions (pthread_barrier*()).

Attachments (1)

0001-Implemented-pthread-barriers.patch (9.5 KB ) - added by hermord 3 years ago.
My implementation of pthread barriers

Download all attachments as: .zip

Change History (13)

comment:1 by luroh, 5 years ago

Milestone: R1Unscheduled

Move POSIX compatibility related tickets out of R1 milestone (FutureHaiku/Features).

comment:2 by korli, 3 years ago

From http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/pthread.h.html

PTHREAD_BARRIER_SERIAL_THREAD
pthread_barrier_t
pthread_barrierattr_t
int   pthread_barrier_destroy(pthread_barrier_t *);
int   pthread_barrier_init(pthread_barrier_t *restrict,
          const pthread_barrierattr_t *restrict, unsigned);
int   pthread_barrier_wait(pthread_barrier_t *);
int   pthread_barrierattr_destroy(pthread_barrierattr_t *);
int   pthread_barrierattr_init(pthread_barrierattr_t *);

are to be declared in headers/posix/pthread.h, implemented in headers/private/libroot/pthread_private.h and src/system/libroot/posix/pthread

pthread barriers are no longer an extension.

Also we shouldn't define PTHREAD_BARRIER_SERIAL_THREAD when we don't support this feature. For instance, CMake 3.7 checks whether PTHREAD_BARRIER_SERIAL_THREAD is defined to import an pthread barrier private implementation.

comment:3 by hermord, 3 years ago

Has a Patch: set

comment:4 by hermord, 3 years ago

For the past two days, I was torn between implementing things from scratch, using pthread_cond and pthread_mutex to keep the code cleaner, and going back to reimplementing them from scratch to avoid wasting memory (sizeof(pthread_barrier_t): 64 bytes (!) vs 20 bytes in current implementation). This is what I settled on.

comment:5 by hermord, 3 years ago

Actually, it was 54 bytes, not 64. Turns out, I can't count.

comment:6 by Barrett, 3 years ago

Owner: changed from nobody to Barrett
Status: newassigned

comment:7 by hermord, 3 years ago

Has a Patch: unset

comment:8 by hermord, 3 years ago

Has a Patch: set

comment:9 by korli, 3 years ago

Seems pretty good to me. A testcase in src/tests/system/libroot/posix would help to see which cases you've tested.

comment:10 by hermord, 3 years ago

I added a test in src/tests/system/libroot/posix/pthread_barrier_test.cpp

comment:11 by korli, 3 years ago

Could you add a blank line at line 5 of pthread_barrier.cpp? The testcase seems OK.

by hermord, 3 years ago

My implementation of pthread barriers

comment:12 by korli, 3 years ago

Resolution: fixed
Status: assignedclosed

Thanks. I edited a little bit (order of includes, commit message and left out a else) and applied in hrev50804.

Note: See TracTickets for help on using tickets.