From 8809e5f42385fdb13365ddd1ef04dc07b3f1a17e Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Tue, 4 Aug 2026 14:51:33 -0500 Subject: [PATCH 1/3] feat(windows): Improve timer performance in c++ and python on Windows --- lib/CMakeLists.txt | 3 +++ lib/espp.cpp | 5 +++++ lib/include/espp.hpp | 33 +++++++++++++++++++++++++++++++++ pc/CMakeLists.txt | 6 ++++++ 4 files changed, 47 insertions(+) diff --git a/lib/CMakeLists.txt b/lib/CMakeLists.txt index dc9e19cdd..637515ac4 100644 --- a/lib/CMakeLists.txt +++ b/lib/CMakeLists.txt @@ -64,6 +64,9 @@ else() set_property(TARGET ${TARGET_NAME} PROPERTY POSITION_INDEPENDENT_CODE ON) target_link_options(${TARGET_NAME} PRIVATE "${LINK_ARG}") target_link_libraries(${TARGET_NAME} ${ESPP_EXTERNAL_LIBS}) + if(WIN32) + target_link_libraries(${TARGET_NAME} winmm) + endif() target_compile_features(${TARGET_NAME} PRIVATE cxx_std_20) # install build output and headers diff --git a/lib/espp.cpp b/lib/espp.cpp index a8bc32434..7642917d3 100644 --- a/lib/espp.cpp +++ b/lib/espp.cpp @@ -1,5 +1,10 @@ #include "espp.hpp" #ifdef _MSC_VER + #pragma comment(lib, "Ws2_32.lib") +#pragma comment(lib, "winmm.lib") + +TimerResolution timer_resolution{}; + #endif diff --git a/lib/include/espp.hpp b/lib/include/espp.hpp index 37eeff76b..4f6b94f67 100644 --- a/lib/include/espp.hpp +++ b/lib/include/espp.hpp @@ -4,6 +4,7 @@ #ifdef _MSC_VER extern "C" { +#include // NOTE: needed for tabulate #include "wcswidth.h" } @@ -64,3 +65,35 @@ extern "C" { #include "state_base.hpp" #include + +#ifdef _MSC_VER + +#include +#include + +// we want to ensure that the timer resolution is set to 1ms, otherwise the +// timer will not be accurate. To do this we need to call timeBeginPeriod(1) at +// the start of the program and timeEndPeriod(1) at the end of the program. +class TimerResolution { + espp::Logger logger{{.tag = "TimerResolution", .level = espp::Logger::Verbosity::INFO}}; + +public: + TimerResolution() { + logger.info("Setting timeBeginPeriod(1)"); + if (timeBeginPeriod(1) == TIMERR_NOERROR) { + logger.info("Success"); + } else { + logger.error("failed to set timeBeginPeriod(1)"); + } + } + ~TimerResolution() { + logger.info("Setting timeEndPeriod(1)"); + timeEndPeriod(1); + } +}; + +// we create a global instance of the TimerResolution class to ensure that the +// timer resolution is set to 1ms for the duration of the program. +extern TimerResolution timer_resolution; + +#endif diff --git a/pc/CMakeLists.txt b/pc/CMakeLists.txt index 494b59bc2..469946607 100644 --- a/pc/CMakeLists.txt +++ b/pc/CMakeLists.txt @@ -28,6 +28,12 @@ MACRO(GEN_TESTS curdir) PRIVATE espp_pc PRIVATE ${ESPP_EXTERNAL_LIBS} ) + if(WIN32) + # need to ensure the whole archive is linked in for windows builds, + # otherwise the windows timer period adjustment code (from espp.hpp) will + # be stripped out and the timer will run at max of 64 hz. + target_link_options(${TEST_NAME} PRIVATE "/WHOLEARCHIVE:espp_pc.lib") + endif() ENDFOREACH() ENDMACRO() From 2d4924cd7f9b838551898f546b1aea8d63ce7693 Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Tue, 4 Aug 2026 16:59:28 -0500 Subject: [PATCH 2/3] fix(windows): Address PR review on Windows timer resolution - espp.hpp: move out of the extern "C" block (only wcswidth, a C header, needs it); a C++ header wrapped in extern "C" can break linkage. - Widen the TimerResolution guard from _MSC_VER to _WIN32 so the 1 ms multimedia-timer resolution also applies to MinGW/clang Windows builds (timeBeginPeriod is available on all Windows toolchains). - Centralize the Windows link libs in CMake: add winmm to ESPP_EXTERNAL_LIBS (WIN32) and drop the MSVC-only #pragma comment(lib, ...) so linkage is consistent across the static lib, tests, and the _espp module and works on all toolchains. - pc/CMakeLists: gate the MSVC-specific /WHOLEARCHIVE flag on MSVC instead of WIN32 so MinGW/clang Windows builds don't receive it. Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/espp.cmake | 8 ++++++-- lib/espp.cpp | 15 +++++++++------ lib/include/espp.hpp | 10 ++++++++-- pc/CMakeLists.txt | 9 +++++---- 4 files changed, 28 insertions(+), 14 deletions(-) diff --git a/lib/espp.cmake b/lib/espp.cmake index 78da945bf..cf79c412d 100644 --- a/lib/espp.cmake +++ b/lib/espp.cmake @@ -89,9 +89,13 @@ if(MSVC) list(APPEND ESPP_SOURCES ${CMAKE_CURRENT_LIST_DIR}/wcswidth.c) endif() -# if we're on Windows, we need to link against ws2_32 +# On Windows link against ws2_32 (sockets) and winmm (timeBeginPeriod, used by +# the TimerResolution helper in espp.hpp). Centralizing these here keeps linkage +# consistent across the static library, tests, and the _espp module, and works +# on all Windows toolchains (MSVC, MinGW, clang) rather than relying on +# MSVC-only #pragma comment(lib, ...). if(WIN32) - set(ESPP_EXTERNAL_LIBS ws2_32) + set(ESPP_EXTERNAL_LIBS ws2_32 winmm) else() set(ESPP_EXTERNAL_LIBS pthread) endif() diff --git a/lib/espp.cpp b/lib/espp.cpp index 7642917d3..5583ab640 100644 --- a/lib/espp.cpp +++ b/lib/espp.cpp @@ -1,10 +1,13 @@ #include "espp.hpp" -#ifdef _MSC_VER - -#pragma comment(lib, "Ws2_32.lib") -#pragma comment(lib, "winmm.lib") - +// The Windows system libraries this needs (Ws2_32, winmm) are linked via CMake +// (see lib/espp.cmake, which sets ESPP_EXTERNAL_LIBS for WIN32), so no +// #pragma comment(lib, ...) is needed here - keeping the link spec in one place +// keeps it consistent across the static library, the tests, and the _espp +// Python module, and works on all Windows toolchains (not just MSVC). + +#ifdef _WIN32 +// Global instance that raises the multimedia timer resolution to 1 ms for the +// lifetime of the program (see TimerResolution in espp.hpp). TimerResolution timer_resolution{}; - #endif diff --git a/lib/include/espp.hpp b/lib/include/espp.hpp index 4f6b94f67..f2de4a6d6 100644 --- a/lib/include/espp.hpp +++ b/lib/include/espp.hpp @@ -3,8 +3,10 @@ #include "socket_msvc.hpp" #ifdef _MSC_VER -extern "C" { +// windows.h is a C++ header and must not be wrapped in extern "C"; only the C +// header (wcswidth) needs it. #include +extern "C" { // NOTE: needed for tabulate #include "wcswidth.h" } @@ -66,7 +68,11 @@ extern "C" { #include -#ifdef _MSC_VER +// The timer-resolution helper uses the Windows multimedia timer API +// (timeBeginPeriod), which is available on all Windows toolchains (MSVC, MinGW, +// clang), so guard on _WIN32 rather than _MSC_VER. winmm is linked via CMake +// (see lib/espp.cmake / pc/CMakeLists.txt). +#ifdef _WIN32 #include #include diff --git a/pc/CMakeLists.txt b/pc/CMakeLists.txt index 469946607..a53dea5d2 100644 --- a/pc/CMakeLists.txt +++ b/pc/CMakeLists.txt @@ -28,10 +28,11 @@ MACRO(GEN_TESTS curdir) PRIVATE espp_pc PRIVATE ${ESPP_EXTERNAL_LIBS} ) - if(WIN32) - # need to ensure the whole archive is linked in for windows builds, - # otherwise the windows timer period adjustment code (from espp.hpp) will - # be stripped out and the timer will run at max of 64 hz. + # /WHOLEARCHIVE is an MSVC/link.exe flag (gate on MSVC, not WIN32, so + # MinGW/clang Windows builds don't receive it). It ensures the whole archive + # is linked in, otherwise the Windows timer-period adjustment code (from + # espp.hpp) is stripped and the timer runs at a max of ~64 Hz. + if(MSVC) target_link_options(${TEST_NAME} PRIVATE "/WHOLEARCHIVE:espp_pc.lib") endif() ENDFOREACH() From 75bac00b6bbc4ec8ef4a7a1a66dbba55f6b2e00c Mon Sep 17 00:00:00 2001 From: William Emfinger Date: Tue, 4 Aug 2026 18:31:13 -0500 Subject: [PATCH 3/3] refactor(windows): Drop debug logging from TimerResolution The TimerResolution helper (which sets the 1 ms multimedia timer resolution on Windows) only used its espp::Logger for bring-up debugging. Now that the timer behavior is verified, remove the logger member and its info/error calls; the class just wraps timeBeginPeriod(1)/timeEndPeriod(1). Co-Authored-By: Claude Opus 4.8 (1M context) --- lib/include/espp.hpp | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/lib/include/espp.hpp b/lib/include/espp.hpp index f2de4a6d6..c1b3fcd0a 100644 --- a/lib/include/espp.hpp +++ b/lib/include/espp.hpp @@ -81,21 +81,9 @@ extern "C" { // timer will not be accurate. To do this we need to call timeBeginPeriod(1) at // the start of the program and timeEndPeriod(1) at the end of the program. class TimerResolution { - espp::Logger logger{{.tag = "TimerResolution", .level = espp::Logger::Verbosity::INFO}}; - public: - TimerResolution() { - logger.info("Setting timeBeginPeriod(1)"); - if (timeBeginPeriod(1) == TIMERR_NOERROR) { - logger.info("Success"); - } else { - logger.error("failed to set timeBeginPeriod(1)"); - } - } - ~TimerResolution() { - logger.info("Setting timeEndPeriod(1)"); - timeEndPeriod(1); - } + TimerResolution() { timeBeginPeriod(1); } + ~TimerResolution() { timeEndPeriod(1); } }; // we create a global instance of the TimerResolution class to ensure that the