Skip to content

add gpu-throttle arg#64

Merged
illuzen merged 3 commits into
mainfrom
illuzen/gpu-throttle
May 22, 2026
Merged

add gpu-throttle arg#64
illuzen merged 3 commits into
mainfrom
illuzen/gpu-throttle

Conversation

@illuzen
Copy link
Copy Markdown
Contributor

@illuzen illuzen commented May 22, 2026

Add GPU throttle option

Adds --gpu-throttle-ms argument to allow users to throttle GPU mining by sleeping between batches. This is useful for reducing GPU utilization to manage temperatures or free up GPU resources for other tasks while still mining.

Usage

# 50ms pause between each GPU batch
./quantus-miner serve --node-addr 127.0.0.1:9833 --gpu-throttle-ms 50

# Or via environment variable
MINER_GPU_THROTTLE_MS=50 ./quantus-miner serve

<!-- CURSOR_SUMMARY -->
---

> [!NOTE]
> **Medium Risk**
> Adds an optional sleep into the GPU mining hot loop and threads the new setting through CLI/service/engine initialization, which can impact mining throughput and cancellation timing if misconfigured.
> 
> **Overview**
> Adds a configurable GPU throttle (`--gpu-throttle-ms` / `MINER_GPU_THROTTLE_MS`) that sleeps between GPU batches to reduce GPU utilization.
> 
> This threads the new setting from `miner-cli` into `miner-service` (`ServiceConfig` + `resolve_gpu_configuration`) and extends `engine-gpu`’s `GpuEngine::try_new`/init to store `throttle_ms` and apply an interruptible delay between `search_range` batches. Docs/examples and GPU benches/examples are updated for the new `GpuEngine::try_new(batch_size, throttle_ms)` signature, and startup logs now report throttle when enabled.
> 
> <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit da3abb0994c840bd3a25bf8deb111c64a8ad047d. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup>
<!-- /CURSOR_SUMMARY -->

Copy link
Copy Markdown

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 2 potential issues.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit 8094eed. Configure here.

Comment thread crates/engine-gpu/src/lib.rs
Comment thread crates/engine-gpu/src/lib.rs
@n13
Copy link
Copy Markdown
Contributor

n13 commented May 22, 2026

PR #64 Review: add gpu-throttle arg

Overall this is a clean, focused change — small surface area, sensible default (0 = off), and the latest commit (6f5597b) already addresses both Bugbot comments. LGTM with a couple of optional nits.

What's good

  • Cancellation-aware sleep ✅ — the throttle now sleeps in throttle_ms / 10 slices and re-checks cancel.is_cancelled() each iteration, returning EngineStatus::Cancelled early. This addresses Bugbot's medium-severity comment about ignored cancellation:
            if self.throttle_ms > 0 && current_start <= range.end {
                let sleep_interval =
                    std::time::Duration::from_millis((self.throttle_ms / 10).max(1));
                let mut remaining = std::time::Duration::from_millis(self.throttle_ms);
                while remaining > std::time::Duration::ZERO {
                    if cancel.is_cancelled() {
                        return EngineStatus::Cancelled {
                            hash_count: total_hashes,
                        };
                    }
                    let sleep_time = remaining.min(sleep_interval);
                    std::thread::sleep(sleep_time);
                    remaining = remaining.saturating_sub(sleep_time);
                }
            }
  • No throttle after final batch ✅ — the current_start <= range.end guard mirrors the outer while condition, so the post-batch sleep is skipped when there are no more batches. This addresses Bugbot's low-severity comment.
  • Plumbing is consistent — CLI flag + MINER_GPU_THROTTLE_MS env + ServiceConfig field, matching the style of existing options (--gpu-batch-size, --gpu-devices, etc.).
  • Logged at init — both gpu_engine ("throttle: {n}ms") and the service startup banner log the value when > 0, so misconfigurations are visible.
  • Benchmarks and verify_nonce example updated to pass 0 — keeps benches free of artificial sleep.

Nits / optional improvements

  1. Sleep slice can grow unbounded. sleep_interval = throttle_ms / 10, so a (silly but possible) --gpu-throttle-ms 10000 becomes 1s polling intervals — that's the cancellation latency upper bound. Consider clamping with an absolute ceiling, e.g.:
let sleep_interval = std::time::Duration::from_millis(
    (self.throttle_ms / 10).clamp(1, 50),
);

50ms is plenty fast for cancellation and keeps the slice count proportional to throttle. Not a blocker — current code is still correct, just a touch lazy at very large values.

  1. README Configuration table doesn't mention the new flag. --gpu-batch-size isn't there either, so it's not a strict requirement — but --gpu-throttle-ms is the kind of "operator knob" users will want to discover. Two lines in README.md would help. (README.md L35–L40)

  2. GpuEngine::try_new(batch_size, throttle_ms) is a breaking signature change. All call sites in-repo are updated, and the crate is internal, so this is fine — just flagging in case there are downstream consumers (e.g. external integration tests) I didn't see.

  3. Per-device, in-parallel throttle (by design). Each GPU worker invokes search_range independently, so with N GPUs all N sleep simultaneously. That matches the stated goal (reduce per-device utilization/temperature), so probably intended — worth a sentence in the PR description if you want to be explicit.

  4. Minor style — per the workspace's preference for inline format args, this is already followed; no action needed.

Verdict

The implementation is correct, the Bugbot feedback was addressed thoughtfully, and the change is small. Optional polish: cap the sleep slice, and a one-line README entry. Otherwise ready to merge once a human reviewer approves.

Copy link
Copy Markdown
Contributor

@n13 n13 left a comment

Choose a reason for hiding this comment

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

GTG!

@illuzen illuzen merged commit 5ce62b3 into main May 22, 2026
6 checks passed
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.

2 participants