On Wed, Jun 17, 2020 at 2:33 PM Andres Freund <and...@anarazel.de> wrote: > > It seems odd and confusing that we have both > > S_LOCK() and s_lock(), anyway. Differentiating functions based on case > > is not great practice. > > It's a terrible idea, yes. Since we don't actually have any non-default > implementations of S_LOCK, perhaps we should just rip it out?
I think we should rip out the conditional nature of the definition and fix the comments. I don't think I prefer getting rid of it completely. But then again on the other hand, what's the point of this crap anyway: #define SpinLockInit(lock) S_INIT_LOCK(lock) #define SpinLockAcquire(lock) S_LOCK(lock) #define SpinLockRelease(lock) S_UNLOCK(lock) #define SpinLockFree(lock) S_LOCK_FREE(lock) This seems like it's straight out of the department of pointless abstraction layers. Maybe we should remove all of the S_WHATEVER() stuff and just define SpinLockAcquire() where we currently define S_LOCK(), SpinLockRelease() where we currently define S_UNLOCK(), etc. And, as you say, make them static inline functions while we're at it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company