From c6af4a245dac9b231906e3e6e73855506dfa154f Mon Sep 17 00:00:00 2001 From: Freddie Akeroyd Date: Wed, 10 Mar 2021 01:17:47 +0000 Subject: [PATCH 1/7] Use waitable timers --- src/libCom/osi/os/WIN32/osdEvent.c | 35 ++++++++++++------ src/libCom/osi/os/WIN32/osdThread.c | 57 ++++++++++++++++++++++------- 2 files changed, 68 insertions(+), 24 deletions(-) diff --git a/src/libCom/osi/os/WIN32/osdEvent.c b/src/libCom/osi/os/WIN32/osdEvent.c index 314138b27..c25ee5f65 100644 --- a/src/libCom/osi/os/WIN32/osdEvent.c +++ b/src/libCom/osi/os/WIN32/osdEvent.c @@ -84,36 +84,49 @@ epicsShareFunc epicsEventStatus epicsEventWait ( epicsEventId pSem ) } } +extern HANDLE osdThreadGetTimer(); + /* * epicsEventWaitWithTimeout () */ epicsShareFunc epicsEventStatus epicsEventWaitWithTimeout ( epicsEventId pSem, double timeOut ) { - static const unsigned mSecPerSec = 1000; + static const unsigned nSec100PerSec = 10000000; + HANDLE handles[2]; DWORD status; - DWORD tmo; + LARGE_INTEGER tmo; + HANDLE timer; if ( timeOut <= 0.0 ) { - tmo = 0u; - } - else if ( timeOut >= INFINITE / mSecPerSec ) { - tmo = INFINITE - 1; + tmo.QuadPart = 0u; } else { - tmo = ( DWORD ) ( ( timeOut * mSecPerSec ) + 0.5 ); - if ( tmo == 0 ) { - tmo = 1; + tmo.QuadPart = -((LONGLONG)(timeOut * nSec100PerSec + 0.5)); // +0.99999999 ? + } + + if (tmo.QuadPart < 0) { + timer = osdThreadGetTimer(); + if (!SetWaitableTimer(timer, &tmo, 0, NULL, NULL, 0)) + { + printf("event error %d\n", GetLastError()); + return epicsEventError; } + handles[0] = pSem->handle; + handles[1] = timer; + status = WaitForMultipleObjects (2, handles, FALSE, INFINITE); + } + else { + status = WaitForSingleObject(pSem->handle, 0); } - status = WaitForSingleObject ( pSem->handle, tmo ); if ( status == WAIT_OBJECT_0 ) { return epicsEventOK; } - else if ( status == WAIT_TIMEOUT ) { + else if ( status == WAIT_OBJECT_0 + 1 || status == WAIT_TIMEOUT ) { return epicsEventWaitTimeout; } else { + printf("event error %d\n", GetLastError()); return epicsEventError; } } diff --git a/src/libCom/osi/os/WIN32/osdThread.c b/src/libCom/osi/os/WIN32/osdThread.c index 8cdb4a3f4..a5c455dac 100644 --- a/src/libCom/osi/os/WIN32/osdThread.c +++ b/src/libCom/osi/os/WIN32/osdThread.c @@ -18,9 +18,6 @@ #define VC_EXTRALEAN #define STRICT -#ifndef _WIN32_WINNT -# define _WIN32_WINNT 0x400 /* No support for W95 */ -#endif #include #include /* for _endthread() etc */ @@ -53,6 +50,7 @@ typedef struct epicsThreadOSD { DWORD id; unsigned epicsPriority; char isSuspended; + HANDLE timer; /* waitable timer */ } win32ThreadParam; typedef struct epicsThreadPrivateOSD { @@ -244,6 +242,8 @@ static void epicsParmCleanupWIN32 ( win32ThreadParam * pParm ) LeaveCriticalSection ( & pGbl->mutex ); CloseHandle ( pParm->handle ); + CloseHandle ( pParm->timer ); + pParm->timer = NULL; free ( pParm ); TlsSetValue ( pGbl->tlsIndexThreadLibraryEPICS, 0 ); } @@ -526,6 +526,11 @@ static win32ThreadParam * epicsThreadParmCreate ( const char *pName ) pParmWIN32->pName = (char *) ( pParmWIN32 + 1 ); strcpy ( pParmWIN32->pName, pName ); pParmWIN32->isSuspended = 0; +#ifdef CREATE_WAITABLE_TIMER_HIGH_RESOLUTION + pParmWIN32->timer = CreateWaitableTimerEx(NULL, NULL, CREATE_WAITABLE_TIMER_HIGH_RESOLUTION, TIMER_ALL_ACCESS); +#else + pParmWIN32->timer = CreateWaitableTimer(NULL, 0, NULL); +#endif } return pParmWIN32; } @@ -764,24 +769,50 @@ epicsShareFunc int epicsShareAPI epicsThreadIsSuspended ( epicsThreadId id ) } } +HANDLE osdThreadGetTimer() +{ + win32ThreadGlobal * pGbl = fetchWin32ThreadGlobal (); + win32ThreadParam * pParm; + + assert ( pGbl ); + + pParm = ( win32ThreadParam * ) + TlsGetValue ( pGbl->tlsIndexThreadLibraryEPICS ); + + return pParm->timer; +} + /* * epicsThreadSleep () */ epicsShareFunc void epicsShareAPI epicsThreadSleep ( double seconds ) { - static const unsigned mSecPerSec = 1000; - DWORD milliSecDelay; + static const unsigned nSec100PerSec = 10000000; + LARGE_INTEGER tmo; + HANDLE timer; - if ( seconds > 0.0 ) { - seconds *= mSecPerSec; - seconds += 0.99999999; /* 8 9s here is optimal */ - milliSecDelay = ( seconds >= INFINITE ) ? - INFINITE - 1 : ( DWORD ) seconds; + if ( seconds <= 0.0 ) { + tmo.QuadPart = 0u; } - else { /* seconds <= 0 or NAN */ - milliSecDelay = 0u; + else { + tmo.QuadPart = -((LONGLONG)(seconds * nSec100PerSec + 0.5)); // +0.99999999 ? + } + + if (tmo.QuadPart == 0) { + Sleep ( 0 ); + } + else { + timer = osdThreadGetTimer(); + if (!SetWaitableTimer(timer, &tmo, 0, NULL, NULL, 0)) + { + printf("timer error %d\n", GetLastError()); + return; + } + if (WaitForSingleObject(timer, INFINITE) != WAIT_OBJECT_0) + { + printf("timer error %d\n", GetLastError()); + } } - Sleep ( milliSecDelay ); } /* From 5f94ab6d9fb914c2bab80368f836d8d8d9b968f4 Mon Sep 17 00:00:00 2001 From: Freddie Akeroyd Date: Wed, 10 Mar 2021 10:47:24 +0000 Subject: [PATCH 2/7] Tidy up --- src/libCom/osi/os/WIN32/osdEvent.c | 13 ++++++------- src/libCom/osi/os/WIN32/osdThread.c | 21 +++++++++++++-------- 2 files changed, 19 insertions(+), 15 deletions(-) diff --git a/src/libCom/osi/os/WIN32/osdEvent.c b/src/libCom/osi/os/WIN32/osdEvent.c index c25ee5f65..9d300e71c 100644 --- a/src/libCom/osi/os/WIN32/osdEvent.c +++ b/src/libCom/osi/os/WIN32/osdEvent.c @@ -84,7 +84,7 @@ epicsShareFunc epicsEventStatus epicsEventWait ( epicsEventId pSem ) } } -extern HANDLE osdThreadGetTimer(); +extern HANDLE osdThreadGetTimer(void); /* from osdThread.c */ /* * epicsEventWaitWithTimeout () @@ -92,7 +92,7 @@ extern HANDLE osdThreadGetTimer(); epicsShareFunc epicsEventStatus epicsEventWaitWithTimeout ( epicsEventId pSem, double timeOut ) { - static const unsigned nSec100PerSec = 10000000; + static const unsigned nSec100PerSec = 10000000u; HANDLE handles[2]; DWORD status; LARGE_INTEGER tmo; @@ -102,14 +102,12 @@ epicsShareFunc epicsEventStatus epicsEventWaitWithTimeout ( tmo.QuadPart = 0u; } else { - tmo.QuadPart = -((LONGLONG)(timeOut * nSec100PerSec + 0.5)); // +0.99999999 ? + tmo.QuadPart = -((LONGLONG)(timeOut * nSec100PerSec + 0.5)); } if (tmo.QuadPart < 0) { timer = osdThreadGetTimer(); - if (!SetWaitableTimer(timer, &tmo, 0, NULL, NULL, 0)) - { - printf("event error %d\n", GetLastError()); + if (!SetWaitableTimer(timer, &tmo, 0, NULL, NULL, 0)) { return epicsEventError; } handles[0] = pSem->handle; @@ -123,10 +121,11 @@ epicsShareFunc epicsEventStatus epicsEventWaitWithTimeout ( return epicsEventOK; } else if ( status == WAIT_OBJECT_0 + 1 || status == WAIT_TIMEOUT ) { + /* WaitForMultipleObjects will trigger WAIT_OBJECT_0 + 1, + WaitForSingleObject will trigger WAIT_TIMEOUT */ return epicsEventWaitTimeout; } else { - printf("event error %d\n", GetLastError()); return epicsEventError; } } diff --git a/src/libCom/osi/os/WIN32/osdThread.c b/src/libCom/osi/os/WIN32/osdThread.c index a5c455dac..1b590de27 100644 --- a/src/libCom/osi/os/WIN32/osdThread.c +++ b/src/libCom/osi/os/WIN32/osdThread.c @@ -528,6 +528,9 @@ static win32ThreadParam * epicsThreadParmCreate ( const char *pName ) pParmWIN32->isSuspended = 0; #ifdef CREATE_WAITABLE_TIMER_HIGH_RESOLUTION pParmWIN32->timer = CreateWaitableTimerEx(NULL, NULL, CREATE_WAITABLE_TIMER_HIGH_RESOLUTION, TIMER_ALL_ACCESS); + if (pParmWIN32->timer == NULL) { + pParmWIN32->timer = CreateWaitableTimer(NULL, 0, NULL); + } #else pParmWIN32->timer = CreateWaitableTimer(NULL, 0, NULL); #endif @@ -769,6 +772,10 @@ epicsShareFunc int epicsShareAPI epicsThreadIsSuspended ( epicsThreadId id ) } } +/** + * osdThreadGetTimer () + * return stored waitable timer object for thread + */ HANDLE osdThreadGetTimer() { win32ThreadGlobal * pGbl = fetchWin32ThreadGlobal (); @@ -787,7 +794,7 @@ HANDLE osdThreadGetTimer() */ epicsShareFunc void epicsShareAPI epicsThreadSleep ( double seconds ) { - static const unsigned nSec100PerSec = 10000000; + static const unsigned nSec100PerSec = 10000000u; LARGE_INTEGER tmo; HANDLE timer; @@ -795,7 +802,7 @@ epicsShareFunc void epicsShareAPI epicsThreadSleep ( double seconds ) tmo.QuadPart = 0u; } else { - tmo.QuadPart = -((LONGLONG)(seconds * nSec100PerSec + 0.5)); // +0.99999999 ? + tmo.QuadPart = -((LONGLONG)(seconds * nSec100PerSec + 0.5)); } if (tmo.QuadPart == 0) { @@ -803,14 +810,12 @@ epicsShareFunc void epicsShareAPI epicsThreadSleep ( double seconds ) } else { timer = osdThreadGetTimer(); - if (!SetWaitableTimer(timer, &tmo, 0, NULL, NULL, 0)) - { - printf("timer error %d\n", GetLastError()); + if (!SetWaitableTimer(timer, &tmo, 0, NULL, NULL, 0)) { + fprintf ( stderr, "epicsThreadSleep: SetWaitableTimer failed %lu\n", GetLastError() ); return; } - if (WaitForSingleObject(timer, INFINITE) != WAIT_OBJECT_0) - { - printf("timer error %d\n", GetLastError()); + if (WaitForSingleObject(timer, INFINITE) != WAIT_OBJECT_0) { + fprintf ( stderr, "epicsThreadSleep: WaitForSingleObject failed %lu\n", GetLastError() ); } } } From c140a0a8047d154c7e94a878e223d8e70cd007c9 Mon Sep 17 00:00:00 2001 From: Freddie Akeroyd Date: Wed, 10 Mar 2021 21:58:35 +0000 Subject: [PATCH 3/7] Add additional cleanup and error handling --- src/libCom/osi/os/WIN32/osdEvent.c | 4 +-- src/libCom/osi/os/WIN32/osdThread.c | 37 +++++++++++++++++--------- src/libCom/osi/os/WIN32/osdThreadPvt.h | 14 ++++++++++ 3 files changed, 41 insertions(+), 14 deletions(-) create mode 100644 src/libCom/osi/os/WIN32/osdThreadPvt.h diff --git a/src/libCom/osi/os/WIN32/osdEvent.c b/src/libCom/osi/os/WIN32/osdEvent.c index 9d300e71c..fb67d6c79 100644 --- a/src/libCom/osi/os/WIN32/osdEvent.c +++ b/src/libCom/osi/os/WIN32/osdEvent.c @@ -26,6 +26,8 @@ #include "shareLib.h" #include "epicsEvent.h" +#include "osdThreadPvt.h" + typedef struct epicsEventOSD { HANDLE handle; } epicsEventOSD; @@ -84,8 +86,6 @@ epicsShareFunc epicsEventStatus epicsEventWait ( epicsEventId pSem ) } } -extern HANDLE osdThreadGetTimer(void); /* from osdThread.c */ - /* * epicsEventWaitWithTimeout () */ diff --git a/src/libCom/osi/os/WIN32/osdThread.c b/src/libCom/osi/os/WIN32/osdThread.c index 1b590de27..0057ee953 100644 --- a/src/libCom/osi/os/WIN32/osdThread.c +++ b/src/libCom/osi/os/WIN32/osdThread.c @@ -30,6 +30,8 @@ #include "ellLib.h" #include "epicsExit.h" +#include "osdThreadPvt.h" + epicsShareFunc void osdThreadHooksRun(epicsThreadId id); void setThreadName ( DWORD dwThreadID, LPCSTR szThreadName ); @@ -227,6 +229,19 @@ static win32ThreadGlobal * fetchWin32ThreadGlobal ( void ) return pWin32ThreadGlobal; } +static void epicsParmCleanupDataWIN32 ( win32ThreadParam * pParm ) +{ + if ( pParm ) { + if ( pParm->handle ) { + CloseHandle ( pParm->handle ); + } + if ( pParm->timer ) { + CloseHandle ( pParm->timer ); + } + free ( pParm ); + } +} + static void epicsParmCleanupWIN32 ( win32ThreadParam * pParm ) { win32ThreadGlobal * pGbl = fetchWin32ThreadGlobal (); @@ -241,10 +256,8 @@ static void epicsParmCleanupWIN32 ( win32ThreadParam * pParm ) ellDelete ( & pGbl->threadList, & pParm->node ); LeaveCriticalSection ( & pGbl->mutex ); - CloseHandle ( pParm->handle ); - CloseHandle ( pParm->timer ); - pParm->timer = NULL; - free ( pParm ); + epicsParmCleanupDataWIN32 ( pParm ); + TlsSetValue ( pGbl->tlsIndexThreadLibraryEPICS, 0 ); } } @@ -528,12 +541,14 @@ static win32ThreadParam * epicsThreadParmCreate ( const char *pName ) pParmWIN32->isSuspended = 0; #ifdef CREATE_WAITABLE_TIMER_HIGH_RESOLUTION pParmWIN32->timer = CreateWaitableTimerEx(NULL, NULL, CREATE_WAITABLE_TIMER_HIGH_RESOLUTION, TIMER_ALL_ACCESS); +#endif if (pParmWIN32->timer == NULL) { pParmWIN32->timer = CreateWaitableTimer(NULL, 0, NULL); } -#else - pParmWIN32->timer = CreateWaitableTimer(NULL, 0, NULL); -#endif + if (pParmWIN32->timer == NULL) { + free(pParmWIN32); + return NULL; + } } return pParmWIN32; } @@ -616,7 +631,7 @@ epicsShareFunc epicsThreadId epicsShareAPI epicsThreadCreate (const char *pName, CREATE_SUSPENDED | STACK_SIZE_PARAM_IS_A_RESERVATION, & threadId ); if ( pParmWIN32->handle == 0 ) { - free ( pParmWIN32 ); + epicsParmCleanupDataWIN32 ( pParmWIN32 ); return NULL; } /* weird win32 interface threadId parameter inconsistency */ @@ -626,8 +641,7 @@ epicsShareFunc epicsThreadId epicsShareAPI epicsThreadCreate (const char *pName, osdPriority = epicsThreadGetOsdPriorityValue (priority); bstat = SetThreadPriority ( pParmWIN32->handle, osdPriority ); if (!bstat) { - CloseHandle ( pParmWIN32->handle ); - free ( pParmWIN32 ); + epicsParmCleanupDataWIN32 ( pParmWIN32 ); return NULL; } @@ -640,8 +654,7 @@ epicsShareFunc epicsThreadId epicsShareAPI epicsThreadCreate (const char *pName, EnterCriticalSection ( & pGbl->mutex ); ellDelete ( & pGbl->threadList, & pParmWIN32->node ); LeaveCriticalSection ( & pGbl->mutex ); - CloseHandle ( pParmWIN32->handle ); - free ( pParmWIN32 ); + epicsParmCleanupDataWIN32 ( pParmWIN32 ); return NULL; } diff --git a/src/libCom/osi/os/WIN32/osdThreadPvt.h b/src/libCom/osi/os/WIN32/osdThreadPvt.h new file mode 100644 index 000000000..9c0e4f0ac --- /dev/null +++ b/src/libCom/osi/os/WIN32/osdThreadPvt.h @@ -0,0 +1,14 @@ +#ifndef osdThreadPvth +#define osdThreadPvth + +#ifdef __cplusplus +extern "C" { +#endif + +extern HANDLE osdThreadGetTimer(void); + +#ifdef __cplusplus +} +#endif + +#endif /* osdThreadPvth */ From 29e9843056bd9c9e65b04c1a50c4e22fd445eac3 Mon Sep 17 00:00:00 2001 From: Freddie Akeroyd Date: Fri, 12 Mar 2021 19:24:09 +0000 Subject: [PATCH 4/7] Add release notes entry for waitable timers --- documentation/RELEASE_NOTES.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/documentation/RELEASE_NOTES.md b/documentation/RELEASE_NOTES.md index 374152589..66d843645 100644 --- a/documentation/RELEASE_NOTES.md +++ b/documentation/RELEASE_NOTES.md @@ -12,6 +12,14 @@ The names of the generated junit xml test output files have been changed from `.xml` to `-results.xml`, to allow better distinction from other xml files. (I.e., for easy wildcard matching.) +### Use waitable timers on Microsoft Windows + +The `epicsEventWaitWithTimeout` and `epicsThreadSleep` functions have +been changed to use waitable timers. On Windows 10 version 1803 or higher +they will use high resolution timers for more consistent timing. + +See https://groups.google.com/a/chromium.org/g/scheduler-dev/c/0GlSPYreJeY +for a comparison of the performance of different timers. ## Changes made between 3.15.7 and 3.15.8 From c78db512f228a524f70d3d13891427b9f13a7f04 Mon Sep 17 00:00:00 2001 From: Ralph Lange Date: Fri, 26 Mar 2021 17:30:36 +0100 Subject: [PATCH 5/7] Windows: use -Z7 instead of -Zi to generate debug symbols "C7 compatible" or "old-style" debug information is kept local in the translation unit (.obj file) and does not create issues with parallel builds --- configure/os/CONFIG.win32-x86.win32-x86 | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/configure/os/CONFIG.win32-x86.win32-x86 b/configure/os/CONFIG.win32-x86.win32-x86 index 6a5078d60..f92856205 100644 --- a/configure/os/CONFIG.win32-x86.win32-x86 +++ b/configure/os/CONFIG.win32-x86.win32-x86 @@ -49,9 +49,9 @@ OPT_CFLAGS_YES_NO = -Ox -Oy- OPT_CFLAGS_YES = $(OPT_CFLAGS_YES_$(OPT_WHOLE_PROGRAM)) # -# -Zi generate program database for debugging information +# -Z7 generate C7 compatible debugging information (inside .obj) # -RTCsu enable run-time error checks -OPT_CFLAGS_NO = -Zi -RTCsu +OPT_CFLAGS_NO = -Z7 -RTCsu # specify object file name and location OBJ_CFLAG = -Fo @@ -116,9 +116,9 @@ OPT_CXXFLAGS_YES_NO = -Ox -Oy- OPT_CXXFLAGS_YES = $(OPT_CXXFLAGS_YES_$(OPT_WHOLE_PROGRAM)) # -# -Zi generate program database for debugging information +# -Z7 generate C7 compatible debugging information (inside .obj) # -RTCsu enable run-time error checks -OPT_CXXFLAGS_NO = -RTCsu -Zi +OPT_CXXFLAGS_NO = -RTCsu -Z7 # specify object file name and location OBJ_CXXFLAG = -Fo From fa069b0845b09b587ae327b6e452a623c4262662 Mon Sep 17 00:00:00 2001 From: Ralph Lange Date: Mon, 29 Mar 2021 17:49:02 +0200 Subject: [PATCH 6/7] Revert config fixes for parallel builds with MSVC This reverts commit 1454f42a2737fd476531a1476653f16ffcf7bb9f. This reverts commit 4aee25e8e2f911c08c8ebaf4245abd54d70bd84c. --- configure/os/CONFIG.win32-x86.win32-x86 | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/configure/os/CONFIG.win32-x86.win32-x86 b/configure/os/CONFIG.win32-x86.win32-x86 index f92856205..c3b780c3e 100644 --- a/configure/os/CONFIG.win32-x86.win32-x86 +++ b/configure/os/CONFIG.win32-x86.win32-x86 @@ -140,20 +140,6 @@ STATIC_LDLIBS_NO= STATIC_LDFLAGS= RANLIB= -# -# option needed for parallel builds with Visual Studio 2013 onward -# VS2012 and above have VisualStudioVersion, so just need to exclude 2012 (11.0) -# -FS Force Synchronous PDB Writes -# -ifneq ($(VisualStudioVersion),) -ifneq ($(VisualStudioVersion),11.0) - OPT_CXXFLAGS_NO += -FS - OPT_CFLAGS_NO += -FS -endif -endif - - -# # add -profile here to run the ms profiler # -LTCG whole program optimization # -incremental:no full linking From 2eb5af31670a2ee9a03eb407102866279b7b7368 Mon Sep 17 00:00:00 2001 From: Andrew Johnson Date: Tue, 4 May 2021 11:18:54 -0500 Subject: [PATCH 7/7] Return dbPutSpecial(paddr, 1) status from dbPut() The status from RSET::special(paddr, 1) has not been returned to the put caller since 3.14, due to a bad up-merge. --- src/ioc/db/dbAccess.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/ioc/db/dbAccess.c b/src/ioc/db/dbAccess.c index e5a63a9c6..7ac56308f 100644 --- a/src/ioc/db/dbAccess.c +++ b/src/ioc/db/dbAccess.c @@ -1290,7 +1290,8 @@ long dbPut(DBADDR *paddr, short dbrType, /* Always do special processing if needed */ if (special) { long status2 = dbPutSpecial(paddr, 1); - if (status2) goto done; + if (status2) + status = status2; } if (status) goto done;