feat(windows): Improve timer performance in c++ and python on Windows

[finger563]

Update the cross-platform library (lib) so that when compiled for Windows, it uses the appropriate Windows API functions to increase the timer resolution, supporting more accurate / higher precision (up to 1ms) for the espp::Timer in both the c++ (pc) tests / library, as well as when used within the espp python library.

[github-actions]

✅Static analysis result - no issues found! ✅

[Copilot]

Pull request overview

This PR updates the PC/lib Windows build path to improve espp::Timer accuracy by increasing Windows timer resolution (targeting 1ms) via WinMM APIs, and adjusts linking so the timer-resolution code is retained in Windows test executables.

Changes:

• Add a Windows-specific timer resolution RAII helper calling timeBeginPeriod(1)/timeEndPeriod(1) and instantiate it for program lifetime.
• Link WinMM (winmm) for the PC static library on Windows and add a Windows link option to keep the full archive when building PC tests.
• Add WinMM linkage hints for MSVC builds in the library source.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File	Description
pc/CMakeLists.txt	Adds Windows-specific whole-archive linking for test executables to retain the timer-resolution object code.
lib/include/espp.hpp	Introduces a Windows/MSVC timer resolution RAII type and a global instance declaration.
lib/espp.cpp	Links WinMM for MSVC and defines the global timer-resolution instance.
lib/CMakeLists.txt	Links winmm for the standalone-built espp_pc static library on Windows.

Suppressed comments (1)

lib/include/espp.hpp:92

• timeEndPeriod(1) should be paired only with a successful timeBeginPeriod(1) call. Calling timeEndPeriod unconditionally can unbalance the global timer-period request count and may fail if timeBeginPeriod didn’t succeed. Track whether the period was successfully set and only call timeEndPeriod when it was.

  ~TimerResolution() {
    logger.info("Setting timeEndPeriod(1)");
    timeEndPeriod(1);
  }

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (7)

lib/include/espp.hpp:99

• The destructor always calls timeEndPeriod(1) even if timeBeginPeriod(1) failed. This can lead to mismatched begin/end calls. Track whether timeBeginPeriod succeeded (e.g., a bool flag set only on TIMERR_NOERROR) and only call timeEndPeriod when it was successfully started; also consider checking/logging the return value of timeEndPeriod for diagnosability.

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);
  }
};

lib/include/espp.hpp:84

• This logs at INFO level on every process start/exit (and potentially on Python module import/unload), which can create noisy output for consumers and tests. Consider lowering these to DEBUG, or only logging on failure (and/or only logging once) while still keeping error logs for unsuccessful calls.

class TimerResolution {
  espp::Logger logger{{.tag = "TimerResolution", .level = espp::Logger::Verbosity::INFO}};

lib/include/espp.hpp:103

• Declaring TimerResolution and timer_resolution in a public header exposes a global symbol as part of the library’s public surface area and increases the risk of name collisions. If the intent is purely internal side effects, prefer keeping both the instance and (ideally) the helper type in a .cpp file with internal linkage, or place them under an internal namespace (e.g., espp::detail) and avoid exporting timer_resolution as a public symbol.

// 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;

lib/include/espp.hpp:8

• <windows.h> is included twice (once under _MSC_VER and again under _WIN32). While include guards prevent functional issues, it adds redundancy and can slow compilation. Consider consolidating the include to a single location (keeping it outside extern \"C\") and only gating it on the platform(s) that require it.

// windows.h is a C++ header and must not be wrapped in extern "C"; only the C
// header (wcswidth) needs it.
#include <windows.h>

lib/include/espp.hpp:78

• <windows.h> is included twice (once under _MSC_VER and again under _WIN32). While include guards prevent functional issues, it adds redundancy and can slow compilation. Consider consolidating the include to a single location (keeping it outside extern \"C\") and only gating it on the platform(s) that require it.

#include <mmsystem.h>
#include <windows.h>

pc/CMakeLists.txt:37

• This explicitly acknowledges that MinGW/clang Windows builds won’t receive the whole-archive behavior, but the underlying issue (the TU with timer_resolution being discarded from a static archive) can still occur with those toolchains as well. To make the feature reliable across Windows toolchains, consider either: (1) adding equivalent whole-archive flags for non-MSVC linkers on WIN32, or (2) refactoring so the timer-resolution code is pulled in via a referenced symbol (e.g., a function called during library initialization) rather than relying on whole-archive behavior.

    # /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()

lib/CMakeLists.txt:69

• winmm is already added to ESPP_EXTERNAL_LIBS on WIN32 in lib/espp.cmake, and this block links it again. Redundant link entries can complicate maintenance/debugging of link lines (especially across targets). Prefer linking winmm in one place (ideally via ESPP_EXTERNAL_LIBS) and remove the duplicate here.

  if(WIN32)
    target_link_libraries(${TARGET_NAME} winmm)
  endif()

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Suppressed comments (4)

lib/CMakeLists.txt:69

• winmm is now already included in ESPP_EXTERNAL_LIBS on WIN32 (see lib/espp.cmake), so linking it again here is redundant and undermines the goal of centralizing Windows system library linkage in one place. Keeping a single source of truth also avoids duplicate-library warnings on some toolchains.

  target_link_libraries(${TARGET_NAME} ${ESPP_EXTERNAL_LIBS})
  if(WIN32)
    target_link_libraries(${TARGET_NAME} winmm)
  endif()

lib/include/espp.hpp:92

• TimerResolution currently ignores the return value from timeBeginPeriod(1) but always calls timeEndPeriod(1) in the destructor. If timeBeginPeriod fails, calling timeEndPeriod can be an unbalanced pair. Also, introducing TimerResolution/timer_resolution in the global namespace from a public umbrella header risks name collisions for consumers; consider scoping it to an internal namespace (e.g. espp::detail).

class TimerResolution {
public:
  TimerResolution() { timeBeginPeriod(1); }
  ~TimerResolution() { timeEndPeriod(1); }
};

lib/espp.cpp:13

• If TimerResolution / timer_resolution are moved under an internal namespace (e.g. espp::detail) in espp.hpp, the definition here also needs to be placed in that same namespace to match the declaration and avoid an ODR/link mismatch.

#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

lib/espp.cpp:12

• timeBeginPeriod(1) changes the system-wide timer resolution for the entire process lifetime (and can affect power consumption / overall system timing behavior). Consider making this opt-in (e.g., enabled when the first espp::Timer is constructed, or behind a build flag / env var) so importing the Python module or linking the library doesn’t automatically change global timing behavior when timers aren’t used.

// 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{};