[x265] [PATCH] restore WINXP_SUPPORT build option, workaround for CONDITION_VARIABLE on XP
Steve Borho
steve at borho.org
Sat Mar 29 22:31:36 CET 2014
On Sat, Mar 29, 2014 at 4:06 PM, Steve Borho <steve at borho.org> wrote:
> # HG changeset patch
> # User Steve Borho <steve at borho.org>
> # Date 1396123561 18000
> # Sat Mar 29 15:06:01 2014 -0500
> # Node ID 5919c3815cf65970e3d2ef8184875954ff888138
> # Parent 9b378e860ddbba528625d56da8915ac05247bffa
> restore WINXP_SUPPORT build option, workaround for CONDITION_VARIABLE on XP
>
> This adapts x264's code for an XP-safe pthread_cond_t to make an XP-safe
> CONDITION_VARIABLE (which was introduced in Windows Vista)
>
> x265 will use native CONDITION_VARIABLE unless the WINXP_SUPPORT cmake option is
> enabled. It forces _WIN32_WINNT=_WIN32_WINNT_VISTA for MinGW for this purpose.
After testing this patch for a while, it seems that MinGW-compiled
x265 is almost 30% slower when we force the target Windows version to
Vista instead of letting it target XP which is its default. I'm not
sure how much of that is caused by this condition variable workaround
or if there is simply some issue with forcing the windows version that
forces MinGW to make some other ugly workarounds.
I'd be interested to hear if others see similar results.
For VC++ builds, targeting XP makes the executable slightly slower but
almost at the noise level.
> diff -r 9b378e860ddb -r 5919c3815cf6 source/CMakeLists.txt
> --- a/source/CMakeLists.txt Thu Mar 27 18:14:55 2014 -0700
> +++ b/source/CMakeLists.txt Sat Mar 29 15:06:01 2014 -0500
> @@ -191,10 +191,16 @@
> set(PLATFORM_LIBS ${PLATFORM_LIBS} ${VLD_LIBRARIES})
> link_directories(${VLD_LIBRARY_DIRS})
> endif()
> - if(MINGW)
> - # MinGW requires a forced Windows version in order to use CONDITION_VARIABLE
> + option(WINXP_SUPPORT "Make binaries compatible with Windows XP" OFF)
> + if(WINXP_SUPPORT)
> + # force use of workarounds for CONDITION_VARIABLE and atomic
> + # intrinsics introduced after XP
> + add_definitions(-D_WIN32_WINNT=_WIN32_WINNT_WINXP)
> + elseif(MINGW)
> + # Unless the user requires XP support, allow MinGW builds to use
> + # native condition variables introduced in Vista
> add_definitions(-D_WIN32_WINNT=_WIN32_WINNT_VISTA)
> - endif(MINGW)
> + endif()
> endif()
>
> include(version) # determine X265_VERSION and X265_LATEST_TAG
> diff -r 9b378e860ddb -r 5919c3815cf6 source/common/CMakeLists.txt
> --- a/source/common/CMakeLists.txt Thu Mar 27 18:14:55 2014 -0700
> +++ b/source/common/CMakeLists.txt Sat Mar 29 15:06:01 2014 -0500
> @@ -145,6 +145,7 @@
> pixel.cpp dct.cpp ipfilter.cpp intrapred.cpp
> cpu.cpp cpu.h version.cpp
> threading.cpp threading.h
> + winxp.h winxp.cpp
> threadpool.cpp threadpool.h
> wavefront.h wavefront.cpp
> md5.cpp md5.h
> diff -r 9b378e860ddb -r 5919c3815cf6 source/common/threading.h
> --- a/source/common/threading.h Thu Mar 27 18:14:55 2014 -0700
> +++ b/source/common/threading.h Sat Mar 29 15:06:01 2014 -0500
> @@ -31,6 +31,7 @@
>
> #ifdef _WIN32
> #include <windows.h>
> +#include "winxp.h" // XP workarounds for CONDITION_VARIABLE and ATOMIC_OR
> #else
> #include <pthread.h>
> #include <semaphore.h>
> @@ -98,29 +99,9 @@
>
> #endif // if !_WIN64
>
> -#if _WIN32_WINNT <= _WIN32_WINNT_WINXP
> -/* Windows XP did not define this intrinsic */
> -FORCEINLINE LONGLONG x265_interlocked_OR64(__inout LONGLONG volatile *Destination,
> - __in LONGLONG Value)
> -{
> - LONGLONG Old;
> -
> - do
> - {
> - Old = *Destination;
> - }
> - while (_InterlockedCompareExchange64(Destination, Old | Value, Old) != Old);
> -
> - return Old;
> -}
> -
> -#define ATOMIC_OR(ptr, mask) x265_interlocked_OR64((volatile LONG64*)ptr, mask)
> -#if defined(_MSC_VER) && !defined(__INTEL_COMPILER)
> -#pragma intrinsic(_InterlockedCompareExchange64)
> +#ifndef ATOMIC_OR
> +#define ATOMIC_OR(ptr, mask) InterlockedOr64((volatile LONG64*)ptr, mask)
> #endif
> -#else // if _WIN32_WINNT <= _WIN32_WINNT_WINXP
> -#define ATOMIC_OR(ptr, mask) InterlockedOr64((volatile LONG64*)ptr, mask)
> -#endif // if _WIN32_WINNT <= _WIN32_WINNT_WINXP
>
> #define CLZ32(id, x) _BitScanReverse(&id, x)
> #define CTZ64(id, x) _BitScanForward64(&id, x)
> @@ -223,6 +204,7 @@
> ~ThreadSafeInteger()
> {
> DeleteCriticalSection(&m_cs);
> + XP_CONDITION_VAR_FREE(&m_cv);
> }
>
> int waitForChange(int prev)
> diff -r 9b378e860ddb -r 5919c3815cf6 source/common/winxp.cpp
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/source/common/winxp.cpp Sat Mar 29 15:06:01 2014 -0500
> @@ -0,0 +1,130 @@
> +/*****************************************************************************
> + * Copyright (C) 2013 x265 project
> + *
> + * Authors: Steve Borho <steve at borho.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02111, USA.
> + *
> + * This program is also available under a commercial proprietary license.
> + * For more information, contact us at licensing at multicorewareinc.com
> + *****************************************************************************/
> +
> +#include "threading.h"
> +#include "winxp.h"
> +#include <windows.h>
> +
> +namespace x265
> +{
> +/* Mmimic CONDITION_VARIABLE functions only supported on Vista+ */
> +
> +#if _WIN32_WINNT <= _WIN32_WINNT_WINXP
> +
> +int WINAPI cond_init(ConditionVariable *cond)
> +{ // InitializeConditionVariable
> + cond->semaphore = CreateSemaphore(NULL, 0, 0x7fffffff, NULL);
> + if (!cond->semaphore)
> + return -1;
> + cond->waitersDone = CreateEvent(NULL, FALSE, FALSE, NULL);
> + if (!cond->waitersDone)
> + return -1;
> +
> + InitializeCriticalSection(&cond->waiterCountMutex);
> + InitializeCriticalSection(&cond->broadcastMutex);
> + cond->waiterCount = 0;
> + cond->bIsBroadcast = false;
> +
> + return 0;
> +}
> +
> +void WINAPI cond_broadcast(ConditionVariable *cond)
> +{ // WakeAllConditionVariable
> + EnterCriticalSection(&cond->broadcastMutex);
> + EnterCriticalSection(&cond->waiterCountMutex);
> + int haveWaiter = 0;
> +
> + if (cond->waiterCount)
> + {
> + cond->bIsBroadcast = 1;
> + haveWaiter = 1;
> + }
> +
> + if (haveWaiter)
> + {
> + ReleaseSemaphore(cond->semaphore, cond->waiterCount, NULL);
> + LeaveCriticalSection(&cond->waiterCountMutex);
> + WaitForSingleObject(cond->waitersDone, INFINITE);
> + cond->bIsBroadcast = 0;
> + }
> + else
> + LeaveCriticalSection(&cond->waiterCountMutex);
> +
> + LeaveCriticalSection(&cond->broadcastMutex);
> +}
> +
> +void WINAPI cond_signal(ConditionVariable *cond)
> +{ // WakeConditionVariable
> + EnterCriticalSection(&cond->broadcastMutex);
> + EnterCriticalSection(&cond->waiterCountMutex);
> + int haveWaiter = cond->waiterCount;
> + LeaveCriticalSection(&cond->waiterCountMutex);
> +
> + if (haveWaiter)
> + {
> + ReleaseSemaphore(cond->semaphore, 1, NULL);
> + WaitForSingleObject(cond->waitersDone, INFINITE);
> + }
> +
> + LeaveCriticalSection(&cond->broadcastMutex);
> +}
> +
> +BOOL WINAPI cond_wait(ConditionVariable *cond, CRITICAL_SECTION *mutex, DWORD wait)
> +{ // SleepConditionVariableCS
> + EnterCriticalSection(&cond->broadcastMutex);
> + EnterCriticalSection(&cond->waiterCountMutex);
> + cond->waiterCount++;
> + LeaveCriticalSection(&cond->waiterCountMutex);
> + LeaveCriticalSection(&cond->broadcastMutex);
> +
> + // unlock the external mutex
> + LeaveCriticalSection(mutex);
> + BOOL ret = WaitForSingleObject(cond->semaphore, wait);
> +
> + EnterCriticalSection(&cond->waiterCountMutex);
> + cond->waiterCount--;
> + int last_waiter = !cond->waiterCount || !cond->bIsBroadcast;
> + LeaveCriticalSection(&cond->waiterCountMutex);
> +
> + if (last_waiter)
> + SetEvent(cond->waitersDone);
> +
> + // lock the external mutex
> + EnterCriticalSection(mutex);
> +
> + // returns false on timeout or error
> + return ret;
> +}
> +
> +/* Native CONDITION_VARIABLE instances are not freed, so this is a special case */
> +void cond_destroy(ConditionVariable *cond)
> +{
> + CloseHandle(cond->semaphore);
> + CloseHandle(cond->waitersDone);
> + DeleteCriticalSection(&cond->broadcastMutex);
> + DeleteCriticalSection(&cond->waiterCountMutex);
> +}
> +
> +#endif // _WIN32_WINNT <= _WIN32_WINNT_WINXP
> +
> +}
> \ No newline at end of file
> diff -r 9b378e860ddb -r 5919c3815cf6 source/common/winxp.h
> --- /dev/null Thu Jan 01 00:00:00 1970 +0000
> +++ b/source/common/winxp.h Sat Mar 29 15:06:01 2014 -0500
> @@ -0,0 +1,90 @@
> +/*****************************************************************************
> + * Copyright (C) 2013 x265 project
> + *
> + * Authors: Steve Borho <steve at borho.org>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02111, USA.
> + *
> + * This program is also available under a commercial proprietary license.
> + * For more information, contact us at licensing at multicorewareinc.com
> + *****************************************************************************/
> +
> +#ifndef X265_WINXP_H
> +#define X265_WINXP_H
> +
> +#if _WIN32_WINNT <= _WIN32_WINNT_WINXP
> +
> +#include <windows.h>
> +
> +namespace x265
> +{
> +
> +/* non-native condition variable */
> +typedef struct
> +{
> + CRITICAL_SECTION broadcastMutex;
> + CRITICAL_SECTION waiterCountMutex;
> + HANDLE semaphore;
> + HANDLE waitersDone;
> + volatile int waiterCount;
> + volatile int bIsBroadcast;
> +} ConditionVariable;
> +
> +int WINAPI cond_init(ConditionVariable *cond);
> +void WINAPI cond_broadcast(ConditionVariable *cond);
> +void WINAPI cond_signal(ConditionVariable *cond);
> +BOOL WINAPI cond_wait(ConditionVariable *cond, CRITICAL_SECTION *mutex, DWORD wait);
> +void cond_destroy(ConditionVariable *cond);
> +
> +/* map missing API symbols to our structure and functions */
> +#define CONDITION_VARIABLE x265::ConditionVariable
> +#define InitializeConditionVariable x265::cond_init
> +#define SleepConditionVariableCS x265::cond_wait
> +#define WakeConditionVariable x265::cond_signal
> +#define WakeAllConditionVariable x265::cond_broadcast
> +#define XP_CONDITION_VAR_FREE x265::cond_destroy
> +
> +#if defined(_MSC_VER)
> +/* Windows XP did not define atomic OR 64, but gcc has a good version, so
> + * only use this workaround when targeting XP with MSVC */
> +FORCEINLINE LONGLONG interlocked_OR64(__inout LONGLONG volatile *Destination,
> + __in LONGLONG Value)
> +{
> + LONGLONG Old;
> +
> + do
> + {
> + Old = *Destination;
> + }
> + while (_InterlockedCompareExchange64(Destination, Old | Value, Old) != Old);
> +
> + return Old;
> +}
> +#define ATOMIC_OR(ptr, mask) x265::interlocked_OR64((volatile LONG64*)ptr, mask)
> +
> +#if defined(_MSC_VER) && !defined(__INTEL_COMPILER)
> +#pragma intrinsic(_InterlockedCompareExchange64)
> +#endif
> +#endif // defined(_MSC_VER)
> +
> +} // namespace x265
> +
> +#else
> +
> +#define XP_CONDITION_VAR_FREE(x)
> +
> +#endif // _WIN32_WINNT <= _WIN32_WINNT_WINXP
> +
> +#endif // ifndef X265_WINXP_H
--
Steve Borho
More information about the x265-devel
mailing list