Skip to content

Added MSVC utime_now implementation #425

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
christian-rauch merged 1 commit into from
Feb 13, 2026
Merged

Added MSVC utime_now implementation #425

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 33 additions & 0 deletions common/time_util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,25 @@ either expressed or implied, of the Regents of The University of Michigan.
#include <math.h>
#include "time_util.h"

#ifdef _MSC_VER

static INIT_ONCE profiler_initd = INIT_ONCE_STATIC_INIT; // static-initialization struct
static volatile LONGLONG profiler_perf_frequency;
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The variable profiler_perf_frequency is declared as 'volatile LONGLONG', but this is unnecessary and potentially confusing. The InitOnceExecuteOnce function provides proper memory barriers to ensure visibility of the initialized value across threads. The volatile keyword doesn't provide the atomicity or memory ordering guarantees needed for multi-threaded access - it only prevents compiler optimizations. Since InitOnceExecuteOnce handles synchronization, the volatile qualifier can be removed.

Suggested change
static volatile LONGLONG profiler_perf_frequency;
static LONGLONG profiler_perf_frequency;

Copilot uses AI. Check for mistakes.

static BOOL __stdcall profiler_init(PINIT_ONCE init_once, PVOID parameter, LPVOID *context)
{
(void) init_once;
(void) parameter;
(void) context;

LARGE_INTEGER freq;
QueryPerformanceFrequency(&freq);
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

QueryPerformanceFrequency can fail (returning FALSE) on systems that don't support a high-resolution performance counter, though this is rare on modern Windows systems. If it fails, freq.QuadPart will be uninitialized, leading to undefined behavior when used in division. Consider checking the return value and handling the failure case appropriately, or at minimum add a comment explaining why this check is omitted if the assumption is that all target systems support this API.

Suggested change
QueryPerformanceFrequency(&freq);
if (!QueryPerformanceFrequency(&freq) || freq.QuadPart == 0) {
/* Fallback to a safe non-zero value if high-resolution counter is unavailable. */
profiler_perf_frequency = 1;
return true;
}

Copilot uses AI. Check for mistakes.
profiler_perf_frequency = freq.QuadPart;
return true;
}

#endif
Comment on lines +32 to +49
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code uses Windows API types and functions (INIT_ONCE, INIT_ONCE_STATIC_INIT, PINIT_ONCE, PVOID, LPVOID, BOOL, LONGLONG, LARGE_INTEGER, QueryPerformanceFrequency, InitOnceExecuteOnce, QueryPerformanceCounter) but doesn't explicitly include windows.h. While these may be available through the indirect inclusion of winsock2.h in time_util.h, this creates a fragile dependency. Consider adding an explicit include for windows.h in the _MSC_VER section to make the dependency clear and avoid potential compilation issues.

Copilot uses AI. Check for mistakes.

struct timeutil_rest
{
int64_t acc_time;
Expand All @@ -48,9 +67,23 @@ void timeutil_rest_destroy(timeutil_rest_t *rest)

int64_t utime_now() // blacklist-ignore
{
#ifdef _MSC_VER

// initialize profiler (will only run once and block for all threads)
InitOnceExecuteOnce(&profiler_initd, profiler_init, NULL, NULL);

LARGE_INTEGER counter;

QueryPerformanceCounter(&counter);
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, QueryPerformanceCounter can fail (returning FALSE) if the system doesn't support a high-resolution performance counter. If it fails, counter.QuadPart will be uninitialized, leading to undefined behavior. Consider checking the return value and handling the failure case appropriately.

Copilot uses AI. Check for mistakes.

// convert to microseconds
return (int64_t) (counter.QuadPart * 1000000LL) / profiler_perf_frequency;
Comment on lines +79 to +80
Copy link

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The arithmetic operation (counter.QuadPart * 1000000LL) / profiler_perf_frequency could overflow for large counter values. QueryPerformanceCounter returns monotonically increasing values that can become very large over time. To prevent overflow, consider reordering the calculation to divide before multiplying, or use a more careful calculation like: (counter.QuadPart / profiler_perf_frequency) * 1000000LL + ((counter.QuadPart % profiler_perf_frequency) * 1000000LL) / profiler_perf_frequency. This preserves precision while avoiding overflow.

Suggested change
// convert to microseconds
return (int64_t) (counter.QuadPart * 1000000LL) / profiler_perf_frequency;
// convert to microseconds without risking overflow in intermediate multiplication
int64_t whole = (int64_t)((counter.QuadPart / profiler_perf_frequency) * 1000000LL);
int64_t frac = (int64_t)(((counter.QuadPart % profiler_perf_frequency) * 1000000LL) / profiler_perf_frequency);
return whole + frac;

Copilot uses AI. Check for mistakes.

#else
struct timeval tv;
gettimeofday (&tv, NULL); // blacklist-ignore
return (int64_t) tv.tv_sec * 1000000 + tv.tv_usec;
#endif
}

int64_t utime_get_seconds(int64_t v)
Expand Down
14 changes: 1 addition & 13 deletions common/time_util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,19 +40,7 @@ either expressed or implied, of the Regents of The University of Michigan.
typedef long long suseconds_t;
#endif

#ifdef _MSC_VER

inline int gettimeofday(struct timeval* tp, void* tzp)
{
(void)tzp;

unsigned long t;
t = time(NULL);
tp->tv_sec = t / 1000;
tp->tv_usec = t % 1000;
return 0;
}
#else
#ifndef _MSC_VER
#include <sys/time.h>
#include <unistd.h>
#endif
Expand Down
Loading