Skip to content

bpo-21302: Add nanosleep() implementation for time.sleep() in Unix - #28526

Closed
Livius90 wants to merge 5 commits into
python:mainfrom
Livius90:unix_nanosleep
Closed

bpo-21302: Add nanosleep() implementation for time.sleep() in Unix#28526
Livius90 wants to merge 5 commits into
python:mainfrom
Livius90:unix_nanosleep

Conversation

@Livius90

@Livius90Livius90 commented Sep 22, 2021

Copy link
Copy Markdown
Contributor
  • Use nanosleep() for time.sleep() in Unix operation systems in case if clock_nanosleep() is not available but nanosleep() is available.

Some previous pull request history can be found here: #28111, #28341, #28483

https://bugs.python.org/issue21302

Replace outdated select() to use nanosleep() for sleep implementation in Unix

@vstinnervstinner left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would prefer to restrict this PR to using nanosleep() in time.sleep(), and nothing else. IMO it's already complex enough :-)

Comment threadModules/_tkinter.c Outdated
Comment threadModules/_multiprocessing/semaphore.c
Comment threadModules/timemodule.c
Comment threadModules/timemodule.c

do {
#ifndef HAVE_CLOCK_NANOSLEEP
#if defined(HAVE_NANOSLEEP) && !defined(HAVE_CLOCK_NANOSLEEP)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The change LGTM, but I propose to write the #ifdef dance differently and rename secs to timeout to make the code more readable. I created PR #28545 based on your PR. Is it better? For the main code in Py_BEGIN_ALLOW_THREADS/Py_END_ALLOW_THREADS, I merged the 3 code paths to put if #ifdef in the Py_BEGIN_ALLOW_THREADS block. What do you think?

@vstinner

Copy link
Copy Markdown
Member

I merged PR #28545.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@Livius90@vstinner@the-knights-who-say-ni@bedevere-bot