On Sat, Jan 9, 2021 at 8:13 AM Thomas Munro <thomas.mu...@gmail.com> wrote:
> On Sun, Jan 3, 2021 at 9:49 AM Fabien COELHO <coe...@cri.ensmp.fr> wrote:
> > > Just for fun, the attached 0002 patch is a quick prototype of a
> > > replacement for that stuff that seems to work OK on a Mac here.  (I'm
> > > not sure if the Windows part makes sense or works.)
> >
> > Thanks! That will definitely help because I do not have a Mac. I'll do
> > some cleanup.
>
> I think the main things to clean up are:

I’m supposed to be on vacation this week, but someone left a shiny new
Arm Mac laptop near me, so here’s a cleaned up version.

> 1.  pthread_barrier_init() should check for errors from
> pthread_cond_init() and pthread_mutex_init(), and return -1.

Done.

> 2.  pthread_barrier_destroy() should call pthread_cond_destroy() and
> pthread_mutex_destroy().

Done.

> 3 . Decide if it's sane for the Windows-based emulation to be in here
> too, or if it should stay in pgbench.c.  Or alternatively, if we're
> emulating pthread stuff on Windows, why not also put the other pthread
> emulation stuff from pgbench.c into a "ports" file; that seems
> premature and overkill for your project.  I dunno.

I decided to solve only the macOS problem for now.  So in this
version, the A and B patches are exactly as you had them in your v7,
except that B includes “port/pg_pthread.h” instead of <pthread.h>.

Maybe it’d make sense to move the Win32 pthread emulation stuff out of
pgbench.c into src/port too (the pre-existing stuff, and the new
barrier stuff you added), but that seems like a separate patch, one
that I’m not best placed to write, and it’s not clear to me that we’ll
want to be using pthread APIs as our main abstraction if/when thread
usage increases in the PG source tree anyway.  Other opinions welcome.

> 4.  cfbot shows that it's not building on Windows because
> HAVE_PTHREAD_BARRIER_WAIT is missing from Solution.pm.

Fixed, I think.

Attachment: v8-0001-A.patch
Description: Binary data

Attachment: v8-0002-Add-missing-pthread_barrier_t.patch
Description: Binary data

Attachment: v8-0003-B.patch
Description: Binary data

Reply via email to