Repository navigation
Wake up a sleeping ipc::Thread on cancel() - #3
Merged
Merged
Conversation
ipc::Thread::sleep() split the sleep of an ipc::Thread into intervals of at most 100 ms to poll the cancellation flag, so a thread that sleeps for a long time woke up at least 10 times per second, and cancel() took effect only at the end of the current interval. Added a mutex and a condition variable to ipc::Thread. cancel() now sets the cancellation flag and signals the condition variable under the lock, and sleep() waits on the condition variable for the whole interval: * POSIX: until a CLOCK_MONOTONIC deadline, or a CLOCK_REALTIME one if pthread_condattr_setclock() fails; the deadline is limited to the maximum value of time_t, so a very long sleep() lasts until cancel(). * MacOS: pthread_condattr_setclock() is not available, so sleep() waits with pthread_cond_timedwait_relative_np() for the time left until a CLOCK_MONOTONIC deadline, recomputed after each wake-up. * Windows: the time left is computed with QueryPerformanceCounter() after each return of SleepConditionVariableCS(), so neither an early timeout nor a spurious wake-up ends sleep() early. sleep() returns STATUS_CANCELLED as soon as the thread is cancelled. sleep() of a thread that is not an ipc::Thread is unchanged. The thread handle, the mutex and the condition variable are kept in the private structure detail::thread_impl_t, defined in Thread.cpp and allocated by the constructor, so ipc/Thread.h no longer includes Windows or pthread headers. The thread launchers became static members of that structure. start() returns STATUS_NO_MEM if the allocation failed. Declared a private copy constructor of ipc::Thread, as the thread now owns the platform-specific state. Added the runtime.ipc.thread_sleep unit test. Co-Authored-By: Claude <noreply@anthropic.com>
Collaborator
|
Hello! I've just merged the changes with some additions. You may test sleeps now. |
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
2 participants
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.
Follow-up to lsp-plugins/lsp-plugins#670.
ipc::Thread::sleep()sleeps in slices of at most 100 ms to checkbCancelled, so a 320 ms sleep of the catalog thread costs four wakeups. With this change,sleep()waits on a condition variable until its deadline andcancel()signals it, so each sleep costs one wakeup andcancel()takes effect at once. Measured with a thread that sleeps 320 ms in a loop like the catalog thread (Linux, 3 runs of 10 s each): 12.4 wakeups/s before, 3.2 after.As you asked,
Thread.hno longer includes Windows headers. The thread handle, the mutex and the condition variable are in a privatedetail::thread_impl_t, defined inThread.cppand allocated by the constructor, asipc::Conditiondoes on Windows.Thread.hno longer includespthread.heither. Nothing in lsp-runtime-lib, lsp-plugin-fw, lsp-ws-lib, lsp-tk-lib, lsp-dsp-units, lsp-plugins-shared or the JACK and PipeWire libraries relied on it, but I can put it back if you prefer.Testing:
runtime.ipc.thread_sleeptest passes, also under valgrind (0 errors). In normal runs the thread typically wakes up 0.05–0.2 ms aftercancel(), and a 320 ms sleep takes one context switch.Happy to rework it however you prefer.
Made with AI assistance. Written by Claude Opus 5.5 under the direction of Chris Simpson (GitHub @CmdrShepardsPie), who reviewed and approved it.