sleep: measure the remaining time against a clock, not the kernel's remainder - #642
Open
cyclistmass wants to merge 1 commit into
Open
cyclistmass wants to merge 1 commit into
cyclistmass wants to merge 1 commit into
Conversation
…emainder (sleep n) returns late, and on a machine that collects often enough it does not return at all, when another thread allocates. This is issue Clozure#639. %nanosleep calls #_nanosleep and, when a signal interrupts the call, sleeps again for the remaining time the kernel wrote back. Each GC suspends the sleeping thread with a signal, and suspend_resume_handler blocks inside the handler until the collection is over. The kernel computes the remaining time before it runs the handler, so the time the thread spends blocked in the handler is not subtracted. Every world stop adds its own duration to the sleep. The error grows with the collection rate. On two 2-vCPU burstable instances a 10 s sleep took 31.8 s on linuxx8664 and 170.1 s on linuxarm64; those are the red halves of the measurement below. Earlier runs on the same arm64 cell were killed at 90 s without returning, so the overrun has no bound that I have established. The fix computes the deadline once, from a clock the Lisp reads itself, and on each EINTR sleeps again for (deadline - now). The kernel's write-back is no longer read at all, so #_nanosleep now gets a NULL rem pointer. The clock is CLOCK_MONOTONIC through #_clock_gettime where the target has it, which is every non-Darwin POSIX target; lib/time.lisp already reads that clock the same way for current-time-in-nanoseconds. On Darwin the deadline comes from gettimeofday, which is a kernel import and needs no entry in the interface database. clock_nanosleep with TIMER_ABSTIME was considered and not used: Darwin does not have it, so it would need a second loop with a different error convention for one target. This form is one loop for every target. The Darwin-only check for a negative, zero-extended remainder goes with the code it guarded: that value is not consulted any more. Cost: one clock_gettime per %nanosleep call and one more per interruption. process-wait polls through %nanosleep once per tick, so this is noise there. Measured red then green with ccl-tests extended-tests/threads/sleep-vs-alloc.lisp on both targets; see the pull request for the cell.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
(sleep n)overruns when another thread allocates, and the error grows with the collection rate. This is issue #639.%nanosleepsleeps again for the remaining time the kernel writes back on EINTR. The kernel computes that remainder before it runs the signal handler, andsuspend_resume_handlerblocks inside the handler until the collection finishes, so the time the thread spends parked is never subtracted. Every world stop adds its own duration to the sleep.There is one definition, under
#-windows-target, with no per-port override, so every non-Windows target has this. Three callers reach it:sleep, the periodic-task sleep inhousekeeping-loop, andprocess-waitonce per poll tick.The patch works the deadline out once, from a clock the Lisp reads itself, then sleeps again for the remaining time it measures on each EINTR. It never reads the kernel write-back, so
#_nanosleepnow gets a NULLrem, and the Darwin negative-remainder workaround goes with the code it guarded.That gives one loop for every target. The clock is
CLOCK_MONOTONICthrough#_clock_gettime, whichlib/time.lispalready reads the same way, andgettimeofdayon Darwin, a kernel import that needs no interface-database entry. I avoidedclock_nanosleepwithTIMER_ABSTIMEbecause Darwin does not have it, which would mean a second loop with a different error convention for one target.Measured
Red then green on both architectures at
6526e21c, changing only this patch, withextended-tests/threads/sleep-vs-alloc.lispfrom Clozure/ccl-tests#10.On arm64 the kernel md5 is identical in both halves, as it should be, since the patch touches no kernel file. Both machines are 2-vCPU burstable instances, so read those as verdicts and not as benchmark figures.
ANSI reports 21679/0 and ccl-tests 243/0 with the patch applied.
I have not tested Darwin. There is no Darwin build here, so the
gettimeofdayarm rests on construction rather than measurement.