8000 gh-125997: ensure that `time.sleep(0)` is not delayed on non-Windows platforms by picnixz · Pull Request #128274 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-125997: ensure that time.sleep(0) is not delayed on non-Windows platforms #128274

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

Closed
wants to merge 26 commits into from
Closed

gh-125997: ensure that time.sleep(0) is not delayed on non-Windows platforms #128274

Changes from 1 commit
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
3754e9e
Ensure that `time.sleep(0)` does not accumulate delays.
picnixz Dec 26, 2024
3c0c4e6
update globals
picnixz Dec 26, 2024
c18f15f
blurb
picnixz Dec 26, 2024
8d270f2
fix Windows compilation
picnixz Dec 26, 2024
2996937
use a fresh `timeval` variable for select(2) portability
picnixz Dec 27, 2024
80de853
ensure that `errno=0` when calling `pysleep[zero_posix]`
picnixz Dec 27, 2024
c7aa428
removing `assert(errno == 0)`
picnixz Dec 27, 2024
f15436e
add errors assertions
picnixz Dec 27, 2024
68e2988
simplify code
picnixz Dec 29, 2024
afc6186
add comments for `time.sleep(0)`
picnixz Dec 30, 2024
f6a817f
improve time.sleep() tests
picnixz Dec 30, 2024
fb9eb41
update `os` scheduling policy docs
picnixz Dec 30, 2024
ecb9636
update docs
picnixz Dec 30, 2024
1ed9877
add What's New
picnixz Dec 30, 2024
23b4740
fix typo
picnixz Jan 2, 2025
40155ff
optimize `time.sleep(0)` path
picnixz Jan 3, 2025
153b326
update docs
picnixz Jan 3, 2025
7e351c9
update tests
picnixz Jan 3, 2025
285c54a
revert un-necessary docs updates
picnixz Jan 3, 2025
434da31
remove performance tests
picnixz Jan 6, 2025
ff06516
implement platform compile-time dependent `time.sleep(0)`
picnixz Jan 6, 2025
40d56f1
address Victor's review
picnixz Jan 7, 2025
c4ce9cc
update blurb
picnixz Jan 7, 2025
54b6dde
simplify `pysleep`
picnixz Jan 7, 2025
405e473
add newline for readability
picnixz Jan 7, 2025
b803045
revert to a `select()` alternative
picnixz Jan 7, 2025
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
Prev Previous commit
Next Next commit
optimize time.sleep(0) path
  • Loading branch information
@picnixz
picnixz committed Jan 3, 2025
commit 40155ffbd597a991b2eb6dbe909b9e9f99cffea7
48 changes: 20 additions & 28 deletions Modules/timemodule.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ module time
/* Forward declarations */
static int pysleep(PyTime_t timeout);
#ifndef MS_WINDOWS
static int pysleep_zero_posix(void);
static int pysleep_zero_posix(void); // see gh-125997
#endif


Expand Down Expand Up @@ -2218,7 +2218,7 @@ pysleep(PyTime_t timeout)
assert(timeout >= 0);
assert(!PyErr_Occurred());
#ifndef MS_WINDOWS
if (timeout == 0) {
if (timeout == 0) { // gh-125997
return pysleep_zero_posix();
}
#endif
Expand Down Expand Up @@ -2408,35 +2408,25 @@ pysleep(PyTime_t timeout)
//
// Rationale
// ---------
// time.sleep(0) accumulates delays if we use nanosleep() or clock_nanosleep().
// To avoid this pitfall, we may either use select(0, NULL, NULL, NULL, &zero)
// or sched_yield(). The former is implementation-sensitive while the latter
// would explicit relinquish the CPU but is more portable [1].
//
// While select() is less portable due to various implementation details
// it is slightly faster [2]. In addition, implicitly calling the kernel's
// algorithm in time.sleep(0) may not be what non-Windows users expect [3].
//
// Therefore, we opt for a solution based on select() instead of sched_yield().
//
// [1] On Linux, calling sched_yield() causes the kernel's scheduling algorithm
// to run as well and could be inefficient in terms of CPU consumption if
// time.sleep(0) is successively called multiple times.
//
// [2] Experimentally, the CPU consumption of a sched_yield() solution is
// similar to that one based on select(), albeit slightly slower.
//
// [3] sched_yield() is not recommended when resources needed by other
// schedulable threads are still held by the caller (which may be
// the case) and using it with with nondeterministic scheduling policies
// such as SCHED_OTHER (which is the default) results in an unspecified
// behaviour.
// time.sleep(0) accumulates delays in the generic implementation, but we can
// skip some calls to `PyTime_Monotonic()` and other checks when the timeout
// is zero. For details, see https://github.com/python/cpython/pull/128274.
static int
pysleep_zero_posix(void)
{
assert(!PyErr_Occurred());

int ret;
int ret, err;
Py_BEGIN_ALLOW_THREADS
#ifdef HAVE_CLOCK_NANOSLEEP
struct timespec zero = {0, 0};
ret = clock_nanosleep(CLOCK_MONOTONIC, TIMER_ABSTIME, &zero, NULL);
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that TIMER_ABSTIME is appropriate here:

Suggested change
ret = clock_nanosleep(CLOCK_MONOTONIC, TIMER_ABSTIME, &zero, NULL);
ret = clock_nanosleep(CLOCK_MONOTONIC, 0, &zero, NULL);

Copy link
Member Author
@picnixz picnixz Jan 7, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I first thought about:

If flags is TIMER_ABSTIME, then t is interpreted as an absolute
time as measured by the clock, clockid. If t is less than or
equal to the current value of the clock, then clock_nanosleep()
returns immediately without suspending the calling thread.

Without this flag, time.sleep(0) takes 50us. Otherwise it takes 2us. So, now I'm more and more inclined to actually revert it back to a select. Because otherwise, it's as if I'm not doing anything (and just skip the call to clock_nanosleep; I mean I already know that the condition on t is verified so it's as if I have no call)

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm leaving for a few days but I'm still struggling to convince myself between the use of select or not. The reason why clock_nanosleep() is slowed down is because we don't pass a zero time struct.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok, I was wrong. See #128274 (comment).

err = ret;
#elif defined(HAVE_NANOSLEEP)
struct timespec zero = {0, 0};
ret = nanosleep(&zero, NULL);
Copy link
Member

Choose a reason for hiding this comment

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

Is it worth it to have 3 code paths for sleep(0)? Can't we always use select()?

Copy link
Member Author

Choose a reason for hiding this comment

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

We had a long discussion on this matter but long story short:

err = errno;
#else
// POSIX-compliant select(2) allows the 'timeout' parameter to
// be modified but also mandates that the function should return
// immediately if *both* structure's fields are zero (which is
Expand All @@ -2447,13 +2437,15 @@ pysleep_zero_posix(void)
// this is also the case for zero timeouts), we prefer supplying
// a fresh timeout everytime.
struct timeval zero = {0, 0};
Py_BEGIN_ALLOW_THREADS
ret = select(0, NULL, NULL, NULL, &zero);
err = errno;
#endif
Py_END_ALLOW_THREADS
if (ret == 0) {
return 0;
}
if (errno != EINTR) {
if (err != EINTR) {
errno = err;
PyErr_SetFromErrno(PyExc_OSError);
return -1;
}
Expand Down
0