Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
135 changes: 135 additions & 0 deletions libgringo/gringo/shared_mutex.hh
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
#pragma once

#if defined(_WIN32) || defined(_WIN64)
Comment thread
rkaminsk marked this conversation as resolved.
#define GRINGO_PLATFORM_WINDOWS 1
#else
#define GRINGO_PLATFORM_WINDOWS 0
#endif
Comment thread
rkaminsk marked this conversation as resolved.

#if !GRINGO_PLATFORM_WINDOWS && (defined(__unix__) || defined(__APPLE__))
#define GRINGO_PLATFORM_POSIX 1
#else
#define GRINGO_PLATFORM_POSIX 0
#endif

#include <cstdio>
#include <cstdlib>
#include <cstring>

#if GRINGO_PLATFORM_WINDOWS
#define WIN32_LEAN_AND_MEAN
#define NOMINMAX
#include <Windows.h>
Comment thread
rkaminsk marked this conversation as resolved.
#elif GRINGO_PLATFORM_POSIX
#include <pthread.h>
#else
#include <shared_mutex>
#endif

namespace Gringo {

#if GRINGO_PLATFORM_POSIX
namespace Detail {

inline void check_pthread_call(char const *fun, int code) noexcept {
if (code != 0) {
std::fprintf(stderr, "fatal: %s failed: %s\n", fun, std::strerror(code));
std::abort();
}
}

} // namespace Detail
#endif

class SharedMutex {
Comment thread
BenKaufmann marked this conversation as resolved.
public:
SharedMutex() noexcept {
#if GRINGO_PLATFORM_WINDOWS
InitializeSRWLock(&srw_);
#elif GRINGO_PLATFORM_POSIX
Detail::check_pthread_call("pthread_rwlock_init", pthread_rwlock_init(&rw_, nullptr));
#endif
}

~SharedMutex() noexcept {
#if GRINGO_PLATFORM_POSIX
Detail::check_pthread_call("pthread_rwlock_destroy", pthread_rwlock_destroy(&rw_));
#endif
Comment thread
rkaminsk marked this conversation as resolved.
}

SharedMutex(const SharedMutex &) = delete;
SharedMutex &operator=(const SharedMutex &) = delete;
SharedMutex(SharedMutex &&) = delete;
SharedMutex &operator=(SharedMutex &&) = delete;

void lock_shared() noexcept {
#if GRINGO_PLATFORM_WINDOWS
AcquireSRWLockShared(&srw_);
#elif GRINGO_PLATFORM_POSIX
Detail::check_pthread_call("pthread_rwlock_rdlock", pthread_rwlock_rdlock(&rw_));
#else
mtx_.lock_shared();
#endif
}

bool try_lock_shared() noexcept {
#if GRINGO_PLATFORM_WINDOWS
return TryAcquireSRWLockShared(&srw_) != 0;
#elif GRINGO_PLATFORM_POSIX
return pthread_rwlock_tryrdlock(&rw_) == 0;
#else
return mtx_.try_lock_shared();
#endif
}

void unlock_shared() noexcept {
#if GRINGO_PLATFORM_WINDOWS
ReleaseSRWLockShared(&srw_);
#elif GRINGO_PLATFORM_POSIX
Detail::check_pthread_call("pthread_rwlock_unlock", pthread_rwlock_unlock(&rw_));
#else
mtx_.unlock_shared();
#endif
}

void lock() noexcept {
#if GRINGO_PLATFORM_WINDOWS
AcquireSRWLockExclusive(&srw_);
#elif GRINGO_PLATFORM_POSIX
Detail::check_pthread_call("pthread_rwlock_wrlock", pthread_rwlock_wrlock(&rw_));
#else
mtx_.lock();
#endif
}

bool try_lock() noexcept {
#if GRINGO_PLATFORM_WINDOWS
return TryAcquireSRWLockExclusive(&srw_) != 0;
#elif GRINGO_PLATFORM_POSIX
return pthread_rwlock_trywrlock(&rw_) == 0;
#else
return mtx_.try_lock();
#endif
}

void unlock() noexcept {
#if GRINGO_PLATFORM_WINDOWS
ReleaseSRWLockExclusive(&srw_);
Comment thread
BenKaufmann marked this conversation as resolved.
#elif GRINGO_PLATFORM_POSIX
Detail::check_pthread_call("pthread_rwlock_unlock", pthread_rwlock_unlock(&rw_));
#else
mtx_.unlock();
#endif
}

private:
#if GRINGO_PLATFORM_WINDOWS
SRWLOCK srw_;
#elif GRINGO_PLATFORM_POSIX
pthread_rwlock_t rw_;
#else
std::shared_timed_mutex mtx_;
#endif
};

} // namespace Gringo
24 changes: 15 additions & 9 deletions libgringo/src/symbol.cc
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,11 @@
#include <algorithm>
#include <cstring>
#include <gringo/hash_set.hh>
#include <gringo/shared_mutex.hh>
#include <gringo/symbol.hh>
#include <iterator>
#include <mutex>
#include <shared_mutex>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that you are using this C++14-header, have you considered using std::shared_timed_mutex instead of the platform-specific implementations?
It's probably not as efficient (on Windows at least), though.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the code path is performance critical, I'd rather stick with the the lightweight locking primitives. I should however use the shared_timed_mutex as the fallback instead of the std::mutex.

Copy link
Copy Markdown
Member Author

@rkaminsk rkaminsk Mar 25, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked at the implementation of std::shared_timed_mutex and at least on linux we could even just use it. The problem is that we support really old macos platforms (10.9) where it is not available. That's why I would simply keep the current implementation. It should be more or less the same as the stl implementation.


#ifdef _MSC_VER
#pragma warning(disable : 4200) // nonstandard extension used: zero-sized array in struct/union
Expand Down Expand Up @@ -82,24 +84,28 @@ template <class T> struct UniqueConstruct {
using Set = hash_set<T, typename T::Hash, typename T::EqualTo>;

template <class U> static T const &construct(U &&x) {
// TODO: in C++17 this can use a read/write lock to not block reading threads
size_t hash = typename T::Hash{}(x);
std::lock_guard<std::mutex> g(mutex_);
auto it = set_.find(x, hash);
if (it != set_.end()) {
return *it;
{
auto lock = std::shared_lock<SharedMutex>{mutex_};
auto it = set_.find(x, hash);
if (it != set_.end()) {
return *it;
}
}
{
auto lock = std::unique_lock<SharedMutex>{mutex_};
return *set_.insert(T{std::forward<U>(x), hash}).first;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

T{std::forward<U>(x) allocates memory, I think you could move this outside unique_lock to have a slight perf benefit as allocation happens outside of mutex. Not sure if the compiler knows to optimize this over the mutex though.

}
return *set_.insert(T{std::forward<U>(x), hash}).first;
}

private:
static Set set_; // NOLINT
static std::mutex mutex_; // NOLINT
static Set set_; // NOLINT
static SharedMutex mutex_; // NOLINT
};

template <class T> typename UniqueConstruct<T>::Set UniqueConstruct<T>::set_; // NOLINT

template <class T> typename std::mutex UniqueConstruct<T>::mutex_; // NOLINT
template <class T> Gringo::SharedMutex UniqueConstruct<T>::mutex_; // NOLINT

template <class T, class U> T const &construct_unique(U &&x) {
return UniqueConstruct<T>::construct(std::forward<U>(x));
Expand Down
Loading