Hi Jakub,
Thanks for the review!
On 7/21/26 12:13, Jakub Jelinek wrote:
External email: Use caution opening links or attachments
On Thu, May 28, 2026 at 09:07:23AM +0100, Matthew Malcomson wrote:
Will follow all above suggestions (apologies about the needless
formatting problems).
+
+static inline void
+gomp_barrier_init (gomp_barrier_t *bar, unsigned count)
+{
+ bar->threadgens
+ = gomp_aligned_alloc (64, sizeof (bar->threadgens[0]) * count);
+ for (unsigned i = 0; i < count; ++i)
+ {
+ bar->threadgens[i].gen = 0;
+ bar->threadgens[i].cgen = 0;
+ }
I'm a little bit worried about this extra threadgens allocation.
Can't it be allocated together with the rest of team?
size_t extra = sizeof (team->ordered_release[0])
+ sizeof (team->implicit_task[0]);
#ifdef GOMP_USE_ALIGNED_WORK_SHARES
team = gomp_aligned_alloc (__alignof (struct gomp_team),
sizeof (*team) + nthreads * extra);
#else
team = team_malloc (sizeof (*team) + nthreads * extra);
#endif
#ifndef HAVE_SYNC_BUILTINS
gomp_mutex_init (&team->work_share_list_free_lock);
#endif
gomp_barrier_init (&team->barrier, nthreads);
gomp_mutex_init (&team->task_lock);
Through some extra inline function add it as another extra,
__alignof (struct gomp_team) is already necessarily 64 byte
aligned because it contains gomp_barrier_t.
There is the reinit case.
W.r.t. allocating together with the rest of the team -- I can easily do
that in the 3rd patch (or after). That because the 3rd patch makes the
`threads_dock` barrier in the pool not use the more complex approach,
and simultaneously removes the reinit case on the more involved barrier.
The `threads_dock` barrier can be a simpler barrier because all re-used
threads are held at the team barrier before release instead of this one.
Hence in the fast path of a re-used team the performance of the
`threads_dock` barrier doesn't have an effect.
The "reinit" case on the more complex barrier is removed because after
the above change the complex barrier is always used for threads in a
team and we have nowhere that the number of threads in a `gomp_team`
structure are changed.
And, does it have to be allocated at all even if nthreads
is really small (say <= 16 or so)?
Has performance been compared even in the low number of threads
cases (i.e. shouldn't we dynamically choose between the hyper strategy
and the linear one)? Not everybody even now has lots of CPUs and
OpenMP is used even in the low number of threads cases, 2, 4, 8, 16...
W.r.t. smaller thread counts: I did test performance of the barrier
approaches on smaller thread counts. On most of the hardware I tested I
found this approach still faster, just not by as much.
That said, while testing isolated implementations of barriers on as much
hardware as possible I did find some hardware where at small thread
counts this "flat" barrier was slower by about 20% (corresponding to
0.2us or similar).
If that question was related to allocation of the barrier I guess you're
thinking more about the case where new teams are being created (i.e. in
nested teams or when changing thread counts). IIRC from other
experiments the cost of those situations is mostly dominated by the cost
of spawning new threads.
-------
Choosing between the hyper strategy and the linear one based on
OMP_WAIT_POLICY (i.e. gomp_spin_count_var) seems reasonable.
Beyond that I don't know of anything to query in order to tell which
approach would be best. The best I can say is "if the hyper approach is
ever better, it's more likely better at high thread counts/spanning
multiple sockets/using SMT" -- but given some `num_threads` and a CPU
layout I wouldn't be able to tell which is likely best.
MM
