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.

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

Jump to bottom

steady_clock not advancing after system resumes #44118

Closed
llvmbot opened this issue Feb 4, 2020 · 3 comments
Closed

steady_clock not advancing after system resumes #44118

llvmbot opened this issue Feb 4, 2020 · 3 comments
Assignees
@ldionne
Labels
bugzilla Issues migrated from bugzilla libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.

Comments

@llvmbot
Copy link
Collaborator

llvmbot commented Feb 4, 2020

Bugzilla Link 44773
Resolution FIXED
Resolved on Oct 13, 2020 07:47
Version unspecified
OS MacOS X
Reporter LLVM Bugzilla Contributor
CC @ldionne,@mclow
Fixed by commit(s) 8bec892

Extended Description

On macOS pre-Catalina we see that in some cases std::steady_clock::now() either reverses or does not advance after the system resumes from sleep. I cannot reproduce this effect on Catalina.

I speculate that pre-Catalina CLOCK_UPTIME_RAW was very close to the raw output of the RDTSC instruction and is therefore reset to zero in some "deeper" sleep states where the CPUs are powered-down. The circumstantial evidence for this is that we were never able to reproduce the issue with short periods of sleep on macs that are plugged in to an electricity supply, whereas it occurred most often in the wild on Monday mornings when people open laptop lids after the weekend.

I'm reporting it here because it seems to me that the current implementation of std::steady_clock does not meet the standard's requirement that it should "advance at a steady rate relative to real time" at least on macOS Mojave.

Workaround

We have addressed this in our application by providing our own steady_clock that's based on CLOCK_MONOTONIC_RAW.

Proposed Fix

I leave it to the libc++ team to decide whether to make a similar change to the standard library - I appreciate that it's a difficult balance given the precision concerns mentioned in https://reviews.llvm.org/D27429

Testing CLOCK_MONOTONIC_RAW on Catalina I do not encounter repeating timestamps at nanosecond resolution so it appears that Apple have addressed that concern mentioned in D27429.

Application-level Symptoms

In our application this manifested in a few ways. The most-obvious was very high CPU usage after resume. When we grabbed a CPU profile it showed that we were burning time in the spinning part of a hybrid lock - it tries to spin for 5 microseconds before suspending. Because std::steady_clock::now() was not advancing it did not ever meet the condition to stop spinning.

We also saw this as bugs where scheduled work was not happening after the desired time in our work scheduler; it would happen much later if at all.
@llvmbot
Copy link
Collaborator Author

llvmbot commented Feb 4, 2020

assigned to @ldionne

@ldionne
Copy link
Member

ldionne commented Feb 10, 2020

I'm somewhat wondering why CLOCK_MONOTONIC_RAW wasn't used in the first place in D27429. It was mentioned, but nobody explained why it wasn't a suitable implementation instead of CLOCK_UPTIME_RAW.

I opened a review (https://reviews.llvm.org/D74341) and we'll find out.

@ldionne
Copy link
Member

ldionne commented Oct 13, 2020

This was fixed with

commit 8bec8927134f116eb11ddea9db78b1a6241884c9
Author: Louis Dionne <ldionne@apple.com>
Date:   Mon Feb 10 18:30:43 2020 +0100

    [libc++][Apple] Use CLOCK_MONOTONIC_RAW instead of CLOCK_UPTIME_RAW for steady_clock

    Summary:
    In D27429, we switched the Apple implementation of steady_clock::now()
    from clock_gettime(CLOCK_MONOTONIC) to clock_gettime(CLOCK_UPTIME_RAW).
    The purpose was to get nanosecond precision, and also to improve the
    performance of the implementation.

    However, it appears that CLOCK_UPTIME_RAW does not satisfy the requirements
    of the Standard, since it is not strictly speaking monotonic. Indeed, the
    clock does not increment while the system is asleep, which had been
    mentioned in D27429 but somehow not addressed.

    This patch switches to CLOCK_MONOTONIC_RAW, which is monotonic, increased
    during sleep, and also has nanosecond precision.

    llvm/llvm-project#44118

@llvmbot llvmbot transferred this issue from llvm/llvm-bugzilla-archive Dec 10, 2021
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ldionne ldionne

Labels
bugzilla Issues migrated from bugzilla libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ldionne @llvmbot