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. |
|
@copilot analyze the build and test failures in CI |
No completed CI builds found for this PR yet. The GitHub status API shows "pending" with 0 status checks. The azdo-helix-failures skill couldn't locate any Azure DevOps builds. Local builds (CoreCLR + Libraries, Mono) all pass successfully. Once CI completes, I can analyze any failures if they occur. |
|
@copilot analyze the build and test failures in CI |
CI Analysis SummaryBuild: 1284159 | Status: Failed | Tests: 26 failed / 3,679,973 passed (99.99% pass rate) Failures Found (All Known Issues - Not PR-Related)1. crossgen2determinism test failure (linux-arm)
2. stackoverflowtester test failure (linux-x64)
Build Analysis Result✅ Build Analysis correctly identified both failures as known issues. RecommendationSAFE TO MERGE - All failures are tracked known issues unrelated to the HAVE_CLOCK_MONOTONIC removal. Local builds pass successfully. The changes don't touch Crossgen2, ReadyToRun, or stack overflow handling code. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
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 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.