The Wayback Machine - https://web.archive.org/web/20210914061831/https://github.com/python/cpython/pull/28111
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

bpo-21302: Add clock_nanosleep() implementation for time.sleep() #28111

Merged
merged 45 commits into from Sep 13, 2021

Conversation

@Livius90
Copy link
Contributor

@Livius90 Livius90 commented Sep 1, 2021

  • Add clock_nanosleep() function to time.sleep() in Unix operation systems.

Requested changes from @vstinner are done. It was runt-time tested in Oracle Linux 8.4 and Windows 10.
Some previous pull request history can be found here: #28077

https://bugs.python.org/issue21302

clock_nanosleep() is available in Linux which has POSIX 2001.12 or newer
Copy link
Member

@vstinner vstinner left a comment

Oh nice, it's much better!

IMO it's worth to mention that time.sleep() has a resolution of 1 nanosecond if the clock_nanosleep() function is available.

Do you know if clock_nanosleep() is available on other platforms than Linux? Is it available on macOS or FreeBSD for example?

configure.ac Outdated Show resolved Hide resolved
Modules/timemodule.c Outdated Show resolved Hide resolved
@Livius90
Copy link
Contributor Author

@Livius90 Livius90 commented Sep 1, 2021

Oh nice, it's much better!

IMO it's worth to mention that time.sleep() has a resolution of 1 nanosecond if the clock_nanosleep() function is available.

Do you know if clock_nanosleep() is available on other platforms than Linux? Is it available on macOS or FreeBSD for example?

I do not think that it is available in macOS and FreeBSD according the gnulib manual, clock_nanosleep() is available sure in Linux from POSIX version 2001.12. So any modern and new Linux based OS has it, moreover it seems to me QNX has also the same clock_nanosleep().

Is there any right place to mention this improved resolution of time.sleep()?

Livius90 added 4 commits Sep 1, 2021
PEP 7 rules appled for _PyTime_AsTimespec and _PyTime_AsTimeval error checking in pysleep().
In calling clock_nanosleep() EINTR is not stored in errno. Need to use return value of clock_nanosleep()/select() for checking it.
@Livius90 Livius90 requested a review from vstinner Sep 1, 2021
blurb-it bot and others added 3 commits Sep 1, 2021
Unix operating systems eg: Linux, macOS, FreeBSD etc. time.sleep() has a resolution of nanoseconds with using clock_nanosleep() or nanosleep() function.
@Livius90
Copy link
Contributor Author

@Livius90 Livius90 commented Sep 1, 2021

In all Unix systems eg: macOS, FreeBSD, Linux etc, nanosleep() is available and as i see there are some codes where select() is used for sleeping microsecs resolution, semaphore.c, _tkinter.c. Sleeping via select() was the state of the art solution in about 1998 in Linux but it is already an outdated solution nowadays. It would be better to use nanosleep() in these codes also.

I will improve it in the rest of codes, too.

Livius90 and others added 2 commits Sep 2, 2021
In all Unix systems eg: macOS, FreeBSD, Linux etc, nanosleep() is available.
@Livius90 Livius90 changed the title bpo-21302: Add clock_nanosleep() implementation for time.sleep() in Linux bpo-21302: Add clock_nanosleep() and nanosleep() implementation for time.sleep() in Unix systems Sep 2, 2021
Livius90 and others added 4 commits Sep 2, 2021
Waitable timer is 100 nsec resolution. Now, seconds to nanosec conversion is limited in usec, soon will come the next developing part to improve it in next commit.
@Livius90 Livius90 changed the title bpo-21302: Add clock_nanosleep() and nanosleep() implementation for time.sleep() in Unix systems bpo-21302: Add clock_nanosleep(), nanosleep() and waitable timer implementation for time.sleep() Sep 3, 2021
Livius90 and others added 4 commits Sep 3, 2021
Waitable timer resolution is 100 nsec but it is limited to 1 usec by round ceiling, moreover sleep for lower then 1 milisec is not possible in Win32 API.
@Livius90
Copy link
Contributor Author

@Livius90 Livius90 commented Sep 3, 2021

Improved time.sleep() is ready for final review. All platforms have now the state of the art solution for sleeping. In Windows it uses Waitable timer object, in Linux it is clock_nanosleep() and in other Unix systems eg: macOS, FreeBSD it is nanosleep() function.

Resolution of Waitable timer object is 100 nsec but it is limited to 1 usec by round ceiling. However, sleeping for lower than 1 millisec is not possible in Win32 API because the overhead of the the event waiting call is more than nanoseconds.

Resolution of clock_nanosleep() and nanosleep() is 1 nsec, clock_nanosleep() is suitable for blocking/waiting until an absolute time date, nanosleep() is suitable only for interval sleeping.

@abalkin, @pganssle and @vstinner please make a review for my final state.

@Livius90
Copy link
Contributor Author

@Livius90 Livius90 commented Sep 10, 2021

PR was changed to add clock_nanosleep() only. Can you review it?

Modules/timemodule.c Outdated Show resolved Hide resolved
Modules/timemodule.c Show resolved Hide resolved
Modules/timemodule.c Show resolved Hide resolved
@Livius90 Livius90 requested a review from vstinner Sep 10, 2021
@vstinner vstinner merged commit 85a4748 into python:main Sep 13, 2021
12 checks passed
12 checks passed
@github-actions
Docs Docs
Details
@github-actions
Check for source changes
Details
@github-actions
Check if generated files are up to date
Details
@github-actions
Windows (x86)
Details
@github-actions
Windows (x64)
Details
@github-actions
macOS
Details
@github-actions
Ubuntu
Details
@github-actions
Ubuntu SSL tests with OpenSSL
Details
@github-actions
Address sanitizer Address sanitizer
Details
Azure Pipelines PR #20210911.35 succeeded
Details
@bedevere-bot
bedevere/issue-number Issue number 21302 found
Details
@bedevere-bot
bedevere/news News entry found in Misc/NEWS.d
@vstinner
Copy link
Member

@vstinner vstinner commented Sep 13, 2021

Merged, thanks! I created PR #28311 follow-up to update the documentation.

@Livius90
Copy link
Contributor Author

@Livius90 Livius90 commented Sep 13, 2021

Thanks the merging.

What are the formal rules to create a new PR to improve Windows implementation, too? Is it good to create a new PR without any bpo issue number? Or can i make more PR (windows implementation and nanosleep()) for this bpo-21302?

@vstinner
Copy link
Member

@vstinner vstinner commented Sep 13, 2021

In this case, you can reuse bpo-21302.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants