Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix macOS and Windows build failures when threading is disabled #1462

Merged
merged 4 commits into from
Jun 26, 2023
Merged
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
52 changes: 42 additions & 10 deletions .github/workflows/ci_workflow.yml
Original file line number Diff line number Diff line change
Expand Up @@ -317,11 +317,12 @@ jobs:
C++${{ matrix.cxx-standard }},
config=${{ matrix.build-type }},
shared=${{ matrix.build-shared }},
cxx=${{ matrix.cxx-standard }}>'
cxx=${{ matrix.cxx-standard }},
threads=${{ matrix.threads-enabled }}>'
runs-on: macos-${{ matrix.osver }}
strategy:
matrix:
build: [1, 2, 3, 4]
build: [1, 2, 3, 4, 5]
include:
# -------------------------------------------------------------------
# VFX CY2023 - C++17 - MacOS 11.0
Expand All @@ -330,26 +331,40 @@ jobs:
- build: 1
build-type: Release
build-shared: 'ON'
threads-enabled: 'ON'
compiler-desc: AppleClang11.0
cxx-standard: 17
osver: 11.0
exclude-tests:
vfx-cy: 23

# Static, Release
# Shared, Release, Threads OFF
- build: 2
build-type: Release
build-shared: 'ON'
threads-enabled: 'OFF'
compiler-desc: AppleClang11.0
cxx-standard: 17
osver: 11.0
exclude-tests:
vfx-cy: 23

# Static, Release
- build: 3
build-type: Release
build-shared: 'OFF'
threads-enabled: 'ON'
compiler-desc: AppleClang11.0
cxx-standard: 17
osver: 11.0
exclude-tests:
vfx-cy: 23

# Shared, Debug
- build: 3
- build: 4
build-type: Debug
build-shared: 'ON'
threads-enabled: 'ON'
compiler-desc: AppleClang11.0
cxx-standard: 17
osver: 11.0
Expand All @@ -360,9 +375,10 @@ jobs:
# VFX CY2023 - C++17 - MacOS 12
# -------------------------------------------------------------------
# Shared, Release
- build: 4
- build: 5
build-type: Release
build-shared: 'ON'
threads-enabled: 'ON'
compiler-desc: AppleClang11.0
cxx-standard: 17
osver: 12.0
Expand Down Expand Up @@ -428,11 +444,12 @@ jobs:
<${{ matrix.compiler-desc }},
config=${{ matrix.build-type }},
shared=${{ matrix.build-shared }},
cxx=${{ matrix.cxx-standard }}>'
cxx=${{ matrix.cxx-standard }},
threads=${{ matrix.threads-enabled }}>'
runs-on: windows-${{ matrix.osver }}
strategy:
matrix:
build: [1, 2, 3, 4]
build: [1, 2, 3, 4, 5]
include:
# -------------------------------------------------------------------
# VFX CY2023 - C++17 - Windows 2022 runner - MSVC 2022 (17.5)
Expand All @@ -441,16 +458,29 @@ jobs:
- build: 1
build-type: Release
build-shared: 'ON'
threads-enabled: 'ON'
compiler-desc: msvc17.5
cxx-standard: 17
vfx-cy: 2023
exclude-tests: ''
osver: 2022

# Static, Release
# Shared, Release, Threads OFF
- build: 2
build-type: Release
build-shared: 'ON'
threads-enabled: 'OFF'
compiler-desc: msvc17.5
cxx-standard: 17
vfx-cy: 2023
exclude-tests: ''
osver: 2022

# Static, Release
- build: 3
build-type: Release
build-shared: 'OFF'
threads-enabled: 'ON'
compiler-desc: msvc17.5
cxx-standard: 17
vfx-cy: 2023
Expand All @@ -461,19 +491,21 @@ jobs:
# VFX CY2022 - C++17 - Windows 2019 runner - MSVC 2019 (16.11)
# -------------------------------------------------------------------
# Shared, Release
- build: 3
- build: 4
build-type: Release
build-shared: 'ON'
threads-enabled: 'ON'
compiler-desc: msvc16.11
cxx-standard: 17
vfx-cy: 2022
exclude-tests: ''
osver: 2019

# Static, Release
- build: 4
- build: 5
build-type: Release
build-shared: 'OFF'
threads-enabled: 'ON'
compiler-desc: msvc16.11
cxx-standard: 17
vfx-cy: 2022
Expand Down
8 changes: 3 additions & 5 deletions src/lib/IlmThread/IlmThreadSemaphore.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,9 @@
//
//-----------------------------------------------------------------------------

#include "IlmThreadConfig.h"
#include "IlmThreadSemaphore.h"

#if !(ILMTHREAD_THREADING_ENABLED)

# include "IlmThreadSemaphore.h"
#if ILMTHREAD_SEMAPHORE_DISABLED

ILMTHREAD_INTERNAL_NAMESPACE_SOURCE_ENTER

Expand All @@ -41,4 +39,4 @@ Semaphore::value () const

ILMTHREAD_INTERNAL_NAMESPACE_SOURCE_EXIT

#endif
#endif // ILMTHREAD_SEMAPHORE_DISABLED
37 changes: 28 additions & 9 deletions src/lib/IlmThread/IlmThreadSemaphore.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,25 +18,41 @@
#include "IlmThreadConfig.h"
#include "IlmThreadNamespace.h"

#if defined(__APPLE__)
# include <AvailabilityMacros.h>
#endif
//
// Decipher the platform-specific threading support.
// Set the ILMTHREAD_SEMAPHORE_* defines to indicate the corresponding
// implementation of the Semaphore class. Only one of these should be
// defined.
//

#if ILMTHREAD_THREADING_ENABLED
# if ILMTHREAD_HAVE_POSIX_SEMAPHORES
# include <semaphore.h>
# elif defined(__APPLE__) && MAC_OS_X_VERSION_MIN_REQUIRED > 1050 && !defined(__ppc__)
# include <dispatch/dispatch.h>
# define ILMTHREAD_SEMAPHORE_POSIX 1
# elif defined(__APPLE__)
# include <AvailabilityMacros.h>
# if MAC_OS_X_VERSION_MIN_REQUIRED > 1050 && !defined(__ppc__)
# include <dispatch/dispatch.h>
# define ILMTHREAD_SEMAPHORE_OSX 1
# else
# include <condition_variable>
# include <mutex>
# define ILMTHREAD_SEMAPHORE_OTHER 1
# endif
# elif (defined(_WIN32) || defined(_WIN64))
# ifdef NOMINMAX
# undef NOMINMAX
# endif
# define NOMINMAX
# include <windows.h>
# define ILMTHREAD_SEMAPHORE_WINDOWS 1
# else
# include <condition_variable>
# include <mutex>
# define ILMTHREAD_SEMAPHORE_OTHER 1
# endif
#else
# define ILMTHREAD_SEMAPHORE_DISABLED 1
#endif

ILMTHREAD_INTERNAL_NAMESPACE_HEADER_ENTER
Expand All @@ -53,18 +69,21 @@ class ILMTHREAD_EXPORT_TYPE Semaphore
ILMTHREAD_EXPORT int value () const;

private:
#if ILMTHREAD_HAVE_POSIX_SEMAPHORES

#if ILMTHREAD_SEMAPHORE_POSIX

mutable sem_t _semaphore;

#elif defined(__APPLE__) && MAC_OS_X_VERSION_MIN_REQUIRED > 1050 && !defined(__ppc__)
#elif ILMTHREAD_SEMAPHORE_OSX

mutable dispatch_semaphore_t _semaphore;

#elif (defined(_WIN32) || defined(_WIN64))
#elif ILMTHREAD_SEMAPHORE_WINDOWS

mutable HANDLE _semaphore;

#elif ILMTHREAD_THREADING_ENABLED
#elif ILMTHREAD_SEMAPHORE_OTHER

//
// If the platform has threads but no semaphores,
// then we implement them ourselves using condition variables
Expand Down
16 changes: 7 additions & 9 deletions src/lib/IlmThread/IlmThreadSemaphoreOSX.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -5,19 +5,18 @@

//-----------------------------------------------------------------------------
//
// class Semaphore -- implementation for OSX platform(it don't support unnamed Posix semaphores)
// class Semaphore -- implementation for OSX platform (it doesn't
// support unnamed Posix semaphores)
//
// std::condition_variable + std::mutex emulation show poor performance
//
//-----------------------------------------------------------------------------

#if defined(__APPLE__) && !ILMTHREAD_HAVE_POSIX_SEMAPHORES
# include <AvailabilityMacros.h>
#include "IlmThreadSemaphore.h"

// No libdispatch prior to 10.6, and no support for it on any ppc.
#if MAC_OS_X_VERSION_MIN_REQUIRED > 1050 && !defined(__ppc__)
#if ILMTHREAD_SEMAPHORE_OSX

# include "Iex.h"
# include "IlmThreadSemaphore.h"
#include "Iex.h"

ILMTHREAD_INTERNAL_NAMESPACE_SOURCE_ENTER

Expand Down Expand Up @@ -63,5 +62,4 @@ Semaphore::value () const

ILMTHREAD_INTERNAL_NAMESPACE_SOURCE_EXIT

# endif
#endif
#endif // ILMTHREAD_SEMAPHORE_OSX
13 changes: 6 additions & 7 deletions src/lib/IlmThread/IlmThreadSemaphorePosix.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,13 @@
//
//-----------------------------------------------------------------------------

#include "IlmThreadConfig.h"
#include "IlmThreadSemaphore.h"

#if ILMTHREAD_HAVE_POSIX_SEMAPHORES
#if ILMTHREAD_SEMAPHORE_POSIX

# include "Iex.h"
# include "IlmThreadSemaphore.h"
# include <assert.h>
# include <errno.h>
#include "Iex.h"
#include <assert.h>
#include <errno.h>

ILMTHREAD_INTERNAL_NAMESPACE_SOURCE_ENTER

Expand Down Expand Up @@ -70,4 +69,4 @@ Semaphore::value () const

ILMTHREAD_INTERNAL_NAMESPACE_SOURCE_EXIT

#endif
#endif // ILMTHREAD_SEMAPHORE_POSIX
15 changes: 3 additions & 12 deletions src/lib/IlmThread/IlmThreadSemaphorePosixCompat.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,19 +10,11 @@
//
//-----------------------------------------------------------------------------

#include "IlmThreadConfig.h"
#include "IlmThreadSemaphore.h"

#if defined(__APPLE__)
# include <AvailabilityMacros.h>
#endif
#if ILMTHREAD_SEMAPHORE_OTHER

// Use this code as a fallback for macOS versions without libdispatch.
#if ILMTHREAD_THREADING_ENABLED
# if (!(ILMTHREAD_HAVE_POSIX_SEMAPHORES) && !defined(_WIN32) && !defined(_WIN64) && \
(!defined(__APPLE__) || (defined(__APPLE__) && \
(MAC_OS_X_VERSION_MIN_REQUIRED < 1060 || defined(__ppc__)))))

# include "IlmThreadSemaphore.h"

ILMTHREAD_INTERNAL_NAMESPACE_SOURCE_ENTER

Expand Down Expand Up @@ -84,5 +76,4 @@ Semaphore::value () const

ILMTHREAD_INTERNAL_NAMESPACE_SOURCE_EXIT

# endif // posix semaphore compat
#endif // enable threading
#endif // ILMTHREAD_SEMAPHORE_OTHER
16 changes: 8 additions & 8 deletions src/lib/IlmThread/IlmThreadSemaphoreWin32.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,14 @@
//
//-----------------------------------------------------------------------------

#include "IlmThreadConfig.h"
#include "IlmThreadSemaphore.h"

#if (defined(_WIN32) || defined(_WIN64)) && !ILMTHREAD_HAVE_POSIX_SEMAPHORES
#if ILMTHREAD_SEMAPHORE_WINDOWS

# include "Iex.h"
# include "IlmThreadSemaphore.h"
# include <assert.h>
# include <iostream>
# include <string>
#include "Iex.h"
#include <assert.h>
#include <iostream>
#include <string>

ILMTHREAD_INTERNAL_NAMESPACE_SOURCE_ENTER

Expand Down Expand Up @@ -126,4 +125,5 @@ Semaphore::value () const

ILMTHREAD_INTERNAL_NAMESPACE_SOURCE_EXIT

#endif // _WIN32
#endif // ILMTHREAD_SEMAPHORE_WINDOWS