Let idle reconciler workers wait without a timeout - #4302
Conversation
|
This pull request changes some projects for the first time in this development cycle. An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch. Git patchFurther information are available in Common Build Issues - Missing version increments. |
131374d to
9ca0f37
Compare
There was a problem hiding this comment.
Pull request overview
Optimizes reconciler workers by eliminating periodic wake-ups while idle and exposing idle state.
Changes:
- Uses untimed waits for clean workers while preserving edit debouncing.
- Adds
AbstractReconciler.isIdle()and lifecycle tests. - Advances the bundle version for the new API.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
AbstractReconciler.java |
Updates worker waiting and adds idle-state API. |
AbstractReconcilerTest.java |
Tests waiting, shutdown, and idle transitions. |
MANIFEST.MF |
Bumps the bundle version to 3.32.0. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
9ca0f37 to
6a25dc3
Compare
There was a problem hiding this comment.
🟡 Changes recommended
Cancellation and repeated installation races can still report idle while reconciliation is about to run or remains active.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Balanced
6a25dc3 to
244b2ec
Compare
The background worker of AbstractReconciler used the same timed wait for idling as for debouncing changes, so every open text editor woke its reconciler thread twice a second with nothing to do. The worker now waits untimed while it is clean and keeps the timed wait only after a change, where it lets further edits coalesce. The startup job keeps its initial delay. The dirty and canceled checks happen under the queue lock that reset() and cancel() notify on, so no wake-up can be lost. Measured with 100 installed idle reconcilers over 10 seconds, the worker threads went from 2000 voluntary context switches and 55 ms of CPU time to none, while the edit-to-process latency stayed at the configured 500 ms delay in both versions. The steady state also becomes observable: the new AbstractReconciler isIdle() reports whether the initial process has run, no change waits and no strategy is active, which is what tests so far reflected into BackgroundWorker for. The reconciler job family only covers startup. Workers canceled by uninstall() stay tracked until their strategy has returned, and a worker only enters a strategy under the lock that cancel() takes, so isIdle() never reports idle while process() still runs or is about to run. Tests cover this: an idle worker must be in WAITING rather than TIMED_WAITING, uninstalling an idle reconciler must end its thread, isIdle() must flip at each stage from startup to uninstall, and it must stay false after an uninstall during process() or initialProcess(), also when a later install and uninstall have happened since. Assisted-by: multiple AI agents and layers of automated tooling 🤖
244b2ec to
a428368
Compare
The background worker of AbstractReconciler used the same timed wait for idling as for debouncing edits, so every open text editor woke its reconciler thread twice a second with nothing to do. The worker now waits untimed while it is clean and keeps the timed wait only after a change, where it lets further edits coalesce; the startup job keeps its initial delay, and the dirty and canceled checks sit under the queue lock that reset() and cancel() notify on, so no wake-up can be lost.
Measured with 100 installed idle reconcilers over 10 seconds, the worker threads went from 2000 voluntary context switches and 55 ms of CPU time to none, while the edit-to-process latency stayed at the configured 500 ms delay.
The steady state also becomes observable from outside: the new AbstractReconciler#isIdle() reports whether the initial process has run, no change waits and no strategy is active. The reconciler job family only covers startup, so tests (JDT's EditorTestHelper among them) so far reflected into BackgroundWorker for this. Three tests cover the change: an idle worker must be in WAITING rather than TIMED_WAITING, uninstalling an idle reconciler must end its thread, and isIdle() must flip at each stage from startup to uninstall.