Skip to content
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.

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

Stopwatch.GetTimestamp() doesn't take into account sleep time on Unix #77945

Open
iSazonov opened this issue Nov 5, 2022 · 7 comments
Open
Labels
area-System.Runtime needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration
Milestone

Comments

@iSazonov
Copy link
Contributor

iSazonov commented Nov 5, 2022

Description

Stopwatch.GetTimestamp() doesn't take into account suspend/sleep time on Unix but does on Windows.

Discovered in PowerShell repo PowerShell/PowerShell#18469

Reproduction Steps

  1. Get Stopwatch.GetTimestamp() value
  2. Sleep/hibernate your notebook
  3. Wait a while and wake up the notebook
  4. Get Stopwatch.GetTimestamp() value

Expected behavior

Stopwatch.GetTimestamp() value takes into account the sleep time.

Actual behavior

Stopwatch.GetTimestamp() value doesn't take into account the sleep time on Unix.

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

Related code

uint64_t SystemNative_GetTimestamp(void)
{
#if HAVE_CLOCK_GETTIME_NSEC_NP
return clock_gettime_nsec_np(CLOCK_UPTIME_RAW);
#else
struct timespec ts;
int result = clock_gettime(CLOCK_MONOTONIC, &ts);
assert(result == 0); // only possible errors are if MONOTONIC isn't supported or &ts is an invalid address
(void)result; // suppress unused parameter warning in release builds
return ((uint64_t)(ts.tv_sec) * SecondsToNanoSeconds) + (uint64_t)(ts.tv_nsec);
#endif
}

uses CLOCK_UPTIME_RAW - the bug seems to be right here. Should it be CLOCK_BOOTTIME?

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@ghost ghost added the untriaged New issue has not been triaged by the area owner label Nov 5, 2022
@ghost
Copy link

ghost commented Nov 5, 2022

Tagging subscribers to this area: @tommcdon
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

Stopwatch.GetTimestamp() doesn't take into account suspend/sleep time on Unix but does on Windows.

Discovered in PowerShell repo PowerShell/PowerShell#18469

Reproduction Steps

  1. Get Stopwatch.GetTimestamp() value
  2. Sleep/hibernate your notebook
  3. Wait a while and wake up the notebook
  4. Get Stopwatch.GetTimestamp() value

Expected behavior

Stopwatch.GetTimestamp() value takes into account the sleep time.

Actual behavior

Stopwatch.GetTimestamp() value doesn't take into account the sleep time on Unix.

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

Related code

uint64_t SystemNative_GetTimestamp(void)
{
#if HAVE_CLOCK_GETTIME_NSEC_NP
return clock_gettime_nsec_np(CLOCK_UPTIME_RAW);
#else
struct timespec ts;
int result = clock_gettime(CLOCK_MONOTONIC, &ts);
assert(result == 0); // only possible errors are if MONOTONIC isn't supported or &ts is an invalid address
(void)result; // suppress unused parameter warning in release builds
return ((uint64_t)(ts.tv_sec) * SecondsToNanoSeconds) + (uint64_t)(ts.tv_nsec);
#endif
}

uses CLOCK_UPTIME_RAW - the bug seems to be right here. Should it be CLOCK_BOOTTIME?

Author: iSazonov
Assignees: -
Labels:

area-System.Diagnostics, untriaged

Milestone: -

@am11
Copy link
Member

am11 commented Nov 5, 2022

uses CLOCK_UPTIME_RAW - the bug seems to be right here.

clock_gettime_nsec_np(CLOCK_UPTIME_RAW is only activated on macOS/FreeBSD, while the linked issue is about Linux.

Should it be CLOCK_BOOTTIME?

We can use CLOCK_BOOTTIME instead of CLOCK_MONOTONIC on Linux if we want to account for the suspend time, like so:

- #if HAVE_CLOCK_GETTIME_NSEC_NP 
+ #if HAVE_CLOCK_GETTIME_NSEC_NP // macOS, FreeBSD
     return clock_gettime_nsec_np(CLOCK_UPTIME_RAW); 
 #else 
     struct timespec ts; 

+ #ifdef CLOCK_BOOTTIME // Linux, Android
+     int result = clock_gettime(CLOCK_BOOTTIME, &ts); 
+ #else // others
     int result = clock_gettime(CLOCK_MONOTONIC, &ts); 
+ #endif

@iSazonov
Copy link
Contributor Author

iSazonov commented Nov 6, 2022

uses CLOCK_UPTIME_RAW - the bug seems to be right here.

clock_gettime_nsec_np(CLOCK_UPTIME_RAW is only activated on macOS/FreeBSD, while the linked issue is about Linux.

For MAC libuv/libuv#2891

@SteveL-MSFT
Copy link
Contributor

Just to be clear, the current behavior which doesn't take into account suspended time doesn't match the uptime command which does.

@tommcdon tommcdon added this to the 8.0.0 milestone Nov 7, 2022
@tommcdon tommcdon removed the untriaged New issue has not been triaged by the area owner label Nov 7, 2022
@tommcdon tommcdon removed this from the 8.0.0 milestone May 11, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label May 11, 2023
@tommcdon tommcdon added area-System.Runtime and removed untriaged New issue has not been triaged by the area owner area-System.Diagnostics labels May 11, 2023
@ghost
Copy link

ghost commented May 11, 2023

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

Issue Details

Description

Stopwatch.GetTimestamp() doesn't take into account suspend/sleep time on Unix but does on Windows.

Discovered in PowerShell repo PowerShell/PowerShell#18469

Reproduction Steps

  1. Get Stopwatch.GetTimestamp() value
  2. Sleep/hibernate your notebook
  3. Wait a while and wake up the notebook
  4. Get Stopwatch.GetTimestamp() value

Expected behavior

Stopwatch.GetTimestamp() value takes into account the sleep time.

Actual behavior

Stopwatch.GetTimestamp() value doesn't take into account the sleep time on Unix.

Regression?

No response

Known Workarounds

No response

Configuration

No response

Other information

Related code

uint64_t SystemNative_GetTimestamp(void)
{
#if HAVE_CLOCK_GETTIME_NSEC_NP
return clock_gettime_nsec_np(CLOCK_UPTIME_RAW);
#else
struct timespec ts;
int result = clock_gettime(CLOCK_MONOTONIC, &ts);
assert(result == 0); // only possible errors are if MONOTONIC isn't supported or &ts is an invalid address
(void)result; // suppress unused parameter warning in release builds
return ((uint64_t)(ts.tv_sec) * SecondsToNanoSeconds) + (uint64_t)(ts.tv_nsec);
#endif
}

uses CLOCK_UPTIME_RAW - the bug seems to be right here. Should it be CLOCK_BOOTTIME?

Author: iSazonov
Assignees: -
Labels:

area-System.Diagnostics, area-System.Runtime

Milestone: -

@ericstj ericstj added the needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration label Aug 14, 2023
@ericstj ericstj added this to the Future milestone Aug 14, 2023
@kilasuit
Copy link

kilasuit commented Dec 2, 2023

This seems like shouldn't be a hard thing to fix and has seemingly looks to have a possible fix identified already above so I hope a PR with a fix will be raised soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Runtime needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration
Projects
None yet
Development

No branches or pull requests

7 participants