Skip to content

Conversation

@alvaro-cuesta
Copy link
Contributor

The current implementation works by setting the current thread's priority, which is inherited by spawned Stockfish workers on Unix but not on Windows, where it had no effect.

Replace the implementation with a process-wide priority change, which is properly inherited on Windows.

Resolves #279

The current implementation works by setting the current thread's
priority, which is inherited by spawned Stockfish workers on Unix
but not on Windows, where it had no effect.

Replace the implementation with a process-wide priority change, which
is properly inherited on Windows.

Resolves lichess-org#279
Copy link
Contributor Author

@alvaro-cuesta alvaro-cuesta left a comment

Choose a reason for hiding this comment

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

Tested and working correctly on:

  • Windows 10
  • Linux 6.6
  • MacOS 26

Comment on lines +454 to +457
// BELOW_NORMAL_PRIORITY_CLASS is the lowest priority that won't completely
// starve tasks of CPU time on high loads. The lowest IDLE_PRIORITY_CLASS
// is stricter than Linux's nice 19!
unsafe { SetPriorityClass(GetCurrentProcess(), BELOW_NORMAL_PRIORITY_CLASS) }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This choice was inherited from thread_priority but I think it makes sense.

IDLE_PRIORITY_CLASS will preempt aggressively and completely starve the process unless the CPU is really idle, which I think could lead to unintended issues (like timing out on API if preempted in the middle of handling a request).

Comment on lines +433 to +438
// On Linux the priority range is -20 (highest) to 19 (lowest). On other
// Unixes the range is -20 to 20.
#[cfg(target_os = "linux")]
const MINIMUM_PRIORITY_NICENESS: libc::c_int = 19;
#[cfg(not(target_os = "linux"))]
const MINIMUM_PRIORITY_NICENESS: libc::c_int = 20;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tested this and Linux clamps to 19, but I still think it makes sense to be explicit.

@niklasf niklasf merged commit 7582ac0 into lichess-org:master Oct 11, 2025
7 checks passed
@niklasf
Copy link
Member

niklasf commented Oct 11, 2025

Thanks!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

--cpu-priority doesn't seem to do anything on Windows

2 participants