Remove HAVE_CLOCK_MONOTONIC and dead fallback code#124120
Remove HAVE_CLOCK_MONOTONIC and dead fallback code#124120
Conversation
Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
|
@copilot Please fix this build break on Windows |
The parameter was always TRUE, so simplify by removing it and always preferring CLOCK_MONOTONIC when available. Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
CLOCK_MONOTONIC is a POSIX constant not available on Windows. Guard the assignment with #ifdef to prevent build failures on Windows/WASM builds. Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
... Fixed. Added |
Deleted all instances of mono_clock_init() and inlined the initialization. On Darwin, the host_get_clock_service call is now directly in mini-posix.c. On Linux/non-Darwin, sampling_clock is already set to CLOCK_MONOTONIC directly. Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
|
@copilot Revert last commit, it does not look like an improvement |
This reverts commit 3ca9315. Keeping the mono_clock_init() wrapper function as requested. Co-authored-by: jkotas <6668460+jkotas@users.noreply.github.com>
Reverted. The |
There was a problem hiding this comment.
Pull request overview
Removes the HAVE_CLOCK_MONOTONIC configuration macro and associated fallback code paths, standardizing on clock_gettime(CLOCK_MONOTONIC, ...) where applicable across CoreCLR PAL, Mono, and native libraries/minipal.
Changes:
- Deleted
HAVE_CLOCK_MONOTONICfrom multiple CMake/config header templates and try-run cache setups. - Simplified runtime code paths to rely on
CLOCK_MONOTONIC(and related pthread clock APIs) without the macro-gated fallbacks. - Updated Mono to guard monotonic usage with
#ifdef CLOCK_MONOTONICto avoid Windows WASM build issues.
Reviewed changes
Copilot reviewed 18 out of 18 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/minipal/time.c | Removes HAVE_CLOCK_MONOTONIC branching and dead #error fallback; always uses clock_gettime(CLOCK_MONOTONIC) when clock_gettime_nsec_np isn’t available. |
| src/native/minipal/minipalconfig.h.in | Drops HAVE_CLOCK_MONOTONIC from generated minipal config header. |
| src/native/minipal/configure.cmake | Removes symbol check and configuration for HAVE_CLOCK_MONOTONIC. |
| src/native/libs/configure.cmake | Removes iOS/tvOS/Android forced HAVE_CLOCK_MONOTONIC and removes the CLOCK_MONOTONIC try-run check. |
| src/native/libs/System.Native/pal_threading.c | Simplifies conditional compilation to rely only on HAVE_PTHREAD_CONDATTR_SETCLOCK. |
| src/native/libs/System.Native/pal_crossprocessmutex.c | Simplifies conditional compilation to rely only on HAVE_PTHREAD_MUTEX_CLOCKLOCK. |
| src/native/libs/Common/pal_config.h.in | Drops HAVE_CLOCK_MONOTONIC from generated native libs PAL config header. |
| src/mono/mono/utils/mono-time.c | Uses #ifdef CLOCK_MONOTONIC instead of HAVE_CLOCK_MONOTONIC to avoid missing constant on some Windows WASM builds. |
| src/mono/mono/eventpipe/ep-rt-mono.c | Removes legacy Apple #undef HAVE_CLOCK_MONOTONIC workaround and gettimeofday() fallback path. |
| src/mono/cmake/configure.cmake | Removes CLOCK_MONOTONIC symbol check. |
| src/mono/cmake/config.h.in | Removes the HAVE_CLOCK_MONOTONIC config define. |
| src/coreclr/pal/src/synchmgr/synchmanager.hpp | Updates GetAbsoluteTimeout signature (removes monotonic preference parameter). |
| src/coreclr/pal/src/synchmgr/synchmanager.cpp | Updates call sites and implementation of GetAbsoluteTimeout; simplifies monotonic selection logic. |
| src/coreclr/pal/src/configure.cmake | Removes CLOCK_MONOTONIC try-run check. |
| src/coreclr/pal/src/config.h.in | Drops HAVE_CLOCK_MONOTONIC from generated PAL config header. |
| eng/native/tryrun_ios_tvos.cmake | Removes cached try-run result for HAVE_CLOCK_MONOTONIC. |
| eng/native/tryrun.cmake | Removes cached try-run results/forced values for HAVE_CLOCK_MONOTONIC. |
| eng/native/tryrun.browser.cmake | Removes cached values for HAVE_CLOCK_MONOTONIC from the browser try-run config. |
Co-authored-by: Aaron R Robinson <arobins@microsoft.com>
|
@copilot analyze the build and test failures in CI |
Changed assert statements to use the simpler form: assert(result == 0 && "message") instead of if (result != 0) { assert(!"message"); }
Co-authored-by: stephentoub <2642209+stephentoub@users.noreply.github.com>
I analyzed the PR and couldn't find completed Azure DevOps CI builds yet. The local builds (CoreCLR + Libraries, Mono) all succeed with the current changes. The PR removes the HAVE_CLOCK_MONOTONIC macro and simplifies code to assume CLOCK_MONOTONIC is available on all Unix platforms. If specific CI failures appear, please share the build URLs or failure details and I can investigate further. |
Description
HAVE_CLOCK_MONOTONIC is always defined as 1 on all Unix-like platforms, so we can remove the macro and all associated fallback code.
Changes Made
Configuration Files
Source Files
#elif HAVE_CLOCK_MONOTONICto#else, removed error fallback, simplified assert statements to use more concise form#if HAVE_PTHREAD_CONDATTR_SETCLOCK(removed&& HAVE_CLOCK_MONOTONIC)fPreferMonotonicClockparameter fromGetAbsoluteTimeout()(was always TRUE)#ifdef HAVE_CLOCK_MONOTONICto#ifdef CLOCK_MONOTONICto guard against Windows WASM builds where the POSIX constant is unavailable#undef HAVE_CLOCK_MONOTONICworkaround andgettimeofday()fallbackAll code now assumes
clock_gettime(CLOCK_MONOTONIC, ...)is available on Unix-like platforms where the CLOCK_MONOTONIC constant is defined.Testing
Net -81 lines across 17 files.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.