Multi-GPU tuning: measure every device, report per device, and stop hiding wrong results#320
Merged
Merged
Conversation
…h counts The statistics line's "Batches per producer" group counted dispatched batches, which are not comparable between producers: one batch yields 2^batchSizeInBits candidates. On a dual-GPU run with batchSizeInBits 24 and 21, batch counts of 1156 vs 595 read as a ~2:1 split while the actual work split was 20:1 (94.0% vs 6.0% of generated candidates) — exactly the question the group exists to answer when more than one device is configured. Reported from a user's dual-GPU logs in discussion #281. Replace it with "Keys per producer", printing each producer's generated candidate count and its share of the total: [Keys per producer: gpu0 (Incremental, GPU)=19 G (94.0%), gpu1 (Random, GPU)=1 G (6.0%)] RuntimeStatistics gains a per-label generated-candidate map alongside the existing batch map. addGeneratedKeys(long) becomes addGeneratedKeys(String, long): the label is required rather than optional so a caller cannot contribute to the lifetime total without naming its producer, which would leave the rendered shares summing to less than the whole. Batch counts are still tracked — TuneConfiguration consumes them — only the rendered field changed. Statistics gains describeProducerShares() and formatCount(), the latter using the same k/M/G thresholds as formatRate() so both read consistently within one line. A zero total omits the share rather than printing NaN%, for the window where a producer is registered but its first batch is still in flight. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…g guide Three user-reported problems from issue #250 and discussion #281, plus the documentation that should have existed alongside TuneConfiguration. 1. Rate and count formatting collapsed precision at every unit boundary (reported in #250). formatRate/formatCount rounded the scaled value to a whole number, so 1.000 and 1.499 G/s both printed as "1 G/s" and tuned runs became impossible to compare once they crossed 1 G/s. The reporter noticed it at G/s, but the same collapse happened at k/s and M/s. Both now keep four significant digits via withSignificantDigits: 1.400 G/s, 130.0 M/s, 2.013 k/s, 412/s. A value within rounding distance of the next unit renders as 1000.0 M/s rather than being promoted; that costs a second format pass to detect the carry, for a band a live rate crosses in one sample, so it is pinned by a test instead. 2. Multi-line reports arrived as one line separated by " | " (reported in #250). The cause is not TuneConfiguration but the CRLF log-injection guard %replace(%msg){'[\r\n]+', ' | '}, present in both logback.xml and examples/logbackConfiguration.xml, so it affected every user. Users were pasting the report into an editor and replacing the separators by hand. util.MultilineLogger emits trusted formatted blocks as one record per line, wired into TuneConfiguration (report), OpenCLContext (device dump) and Main (transformed-configuration echo). This does not weaken the guard: the property it defends is that every physical line carries a prefix the appender wrote, and splitting preserves that because the appender stamps each record itself. The split consumes every line-break form so no line can carry another past it, and each line is passed as an argument rather than as the format string. Called out for Main in particular because its YAML rendering, unlike the JSON one, can carry a real line break out of a user-supplied value such as vanityPattern. 3. TuneConfiguration printed a bare 0.00 for arms that ran without error but completed no batch inside the measurement window, which is indistinguishable from a driver rejection at a glance. On an integrated GPU, batchSizeInBits=22 with keysPerWorkItem=1 needs ~40 s against a 20 s window. ArmResult gains tooSlowToMeasure() and both the report table and the per-arm log line now say NOT MEASURED with the reason and the remedy. Also adds docs/tuning-your-gpu.md, a step-by-step guide linked from the README: device indices, config fields, capturing the log, reading the report, tuning a second GPU, contributing measurements, troubleshooting. Two things in it are new documentation rather than restatement — the integrated-GPU contention effect (an Arc iGPU measured 16.3 M keys/s alone and 5.5 M/s alongside a discrete card, because it shares system RAM with the host) and the truncated-sweep trap (a winner sitting at the largest swept keysPerWorkItem means the real optimum is higher). Architecture rule: "Cli" is added to the Util layer's accessor list, because Main now logs through util.MultilineLogger. Cli is the topmost layer, so the dependency is downward and introduces no cycle; its earlier absence was incidental rather than a deliberate restriction. README fix: the TuneConfiguration section claimed "25 arms x 25 s ~ 10 minutes". The default candidate lists are 7 x 5 = 35 arms, about 15 minutes, as confirmed by "Arm 21/35" in a user-supplied log. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two problems, found while registering a contributor's ThinkPad with an RTX 500 Ada and an integrated Arc Pro for discussion #281. detect_gpu() returned as soon as nvidia-smi named anything, which made the platform-wide enumeration below it unreachable on exactly the machines that need it most: a laptop with an NVIDIA card and an integrated GPU recorded only the NVIDIA. The contributor's Arc — a device this project runs on, and one whose measurements we specifically wanted — was silently dropped. All sources are now consulted and merged, with duplicates removed, including the case where one source's name is contained in the other's. The field itself was a single string, with multiple GPUs represented by joining them with ", " (one existing entry did this). Multi-GPU machines are the normal case here rather than the exception, so gpu is now a real JSON list: readers no longer have to split on a separator that a device name could itself contain. Both existing registry entries are migrated. --set takes a comma-separated value for list fields, so a correction stays one flag: register_machine.py --set gpu="RTX 500 Ada,Arc Pro Graphics" The "keep a manually supplied value" path now treats an empty list as "nothing detected", so a hand-curated gpu list survives a re-run on a machine where detection comes up empty, matching how the storage string already behaved. plot.py reads only cpu.l3_mb from the registry, so nothing downstream is affected. Verified locally: detection went from one GPU to two, --set with two values round-trips through a write and a re-run, and machines.json stays valid. Documented in docs/measurements/README.md and docs/tuning-your-gpu.md, both of which now also warn that detection reports what the operating system enumerates, which need not match the OpenCL device list — OpenCLInfo remains authoritative for which devices this tool can actually drive. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Adds coreultra7155h-95g-win11 (Lenovo ThinkPad P14s Gen 5: Core Ultra 7 155H, 24 MB L3, 95.5 GB RAM, JDK 26.0.1, Crucial P3 Plus 4TB NVMe) carrying both an NVIDIA RTX 500 Ada and an integrated Intel Arc Pro, plus tuner_coreultra7155h.csv with all 84 measured arms across the two devices. Contributed through discussion 281. Every machine registered so far has been single-GPU high-end desktop hardware, so this is the first data point with a laptop discrete card, the first with an integrated GPU, and the first where the same host drives two devices that interfere with each other. Measured peaks: 110,725,506 candidates/s on the RTX at batchSizeInBits=24, keysPerWorkItem=2048, and 36,481,092 candidates/s on the Arc at 23, 2048. Both sweeps ran 20 s arms with a 5 s warmup against a 140,964,881-entry target, with verification cost measured on the contributor's own storage (10.69 and 10.11 us). The rows were parsed out of the submitted logs rather than transcribed. The gpu list in the registry entry was completed by hand: the contributor's run predates the detect_gpu fix in the previous commit, so the script reported only the NVIDIA and omitted the Arc entirely. The entry's notes record that, along with the contention effect measured on this machine — the integrated GPU loses about 3x when the discrete card runs alongside it, because it shares system RAM with the host. Also restores REUSE compliance, which had been failing on main and which this commit would otherwise have pushed one file further from green. Measurement data and generated plots are covered by globs rather than by name, so future machines and benchmark runs do not each need an edit — that is how these drifted out of coverage. Four example configs that were never added to the existing list are included too. Verified: 486/486 files compliant, was 462/486. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ement
A producer that died part-way through its measurement window was reported with
whatever throughput it managed before dying, sitting in the report next to
healthy arms with nothing to distinguish it.
Found in a contributor's Intel Arc sweep in discussion 281. The
batchSizeInBits=19, keysPerWorkItem=8192 arm's producer hit CL_OUT_OF_RESOURCES
seconds into its window and stopped; the report presented "1,022,288
candidates/s" for it. The only tell was an addresses-checked rate of 0.00, which
is easy to read past. Worse, that arm immediately preceded a cascade in which
every larger grid failed with CL_OUT_OF_HOST_MEMORY — including combinations
that had measured fine an hour earlier — so the one row that would have
identified the cause was the one dressed up as normal.
The exception never had a way out of the producer: run() catches it on the
executor thread and logs it, and ProducerState.NOT_RUNNING cannot distinguish
"asked to stop" from "died". Whoever started the producer therefore saw only a
producer that had stopped producing.
Producers now record what terminated their run loop, exposed through
ProducerStateProvider#getTerminalFailure(), and TuneConfiguration consults it
after the window instead of trusting the rate. Such an arm is recorded as FAILED
with the cause. The partial rate is discarded rather than reported beside the
failure: how much of the window the producer survived is unknown, so keeping it
would invite exactly the comparison it cannot support.
Written test-first. Both tests were confirmed red — getTerminalFailure() and
armResultFor() did not exist — before either was implemented:
AbstractProducerTest
getTerminalFailure_exceptionInProduceKeys_returnsTheExceptionThatStoppedTheLoop
getTerminalFailure_producerRanWithoutError_returnsNull
TuneConfigurationTest
armResultFor_producerDiedDuringTheWindow_isRecordedAsFailedNotAsAMeasurement
armResultFor_producerSurvivedTheWindow_keepsTheMeasuredRate
The second test of each pair guards the obvious over-correction: a clean
shutdown must not be reported as a failure, and a surviving producer must keep
its measured rate.
Not fixed here: why the device stays wedged after the first CL_OUT_OF_RESOURCES,
such that grids which worked minutes earlier then fail. That is device or driver
state we cannot diagnose from a log, and possibly not ours at all. What this
change does is stop the sweep from hiding the arm that caused it.
Verified: 81 tests across AbstractProducerTest, TuneConfigurationTest,
ProducerStateTest, FinderTest and the architecture rules.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous commit fixed the wrong failure mode. It tracked producers that
die, and the Intel Arc case in discussion 281 is not one: ProducerOpenCL
catches a CLException per secret, logs it and continues, so the producer never
died and getTerminalFailure() stayed null. The arm that started the whole
investigation was still reported as a measurement.
Re-reading the submitted log made this unambiguous. All 40 CL_OUT_OF_RESOURCES
fall between 16:45:02,518 and 16:45:22,496, entirely inside arm 8, which the
sweep closed at 16:45:22,999 and recorded as 1,022,288 candidates/s. The
producer was alive and failing for the whole window. The 60 CL_OUT_OF_HOST_MEMORY
of the later arms come from initProducer() on the main thread, propagate
normally, and were already reported correctly before either commit.
So a producer has two failure paths and neither reached the thread that started
it:
fatal - escapes produceKeys(); run() logs it and leaves the loop, and
ProducerState.NOT_RUNNING cannot say whether it was asked to stop
or died
swallowed - caught per secret, logged, loop continues; the producer stays
alive and looks entirely healthy while producing nothing
getTerminalFailure() is therefore replaced by getLastFailure(), recorded at both
funnels: run()'s catch, and logErrorInProduceKeys(...), which is the single point
every swallowed failure already passed through. Only the most recent throwable is
kept — a 20-second failure storm reports the same cause repeatedly, and what
callers act on is whether anything went wrong at all.
Written test-first again. The new test was confirmed red before the fix:
AbstractProducerTest
getLastFailure_secretFailureSwallowed_isRecordedAlthoughTheProducerKeepsRunning
It asserts the producer is still running after the swallowed failure, which is
exactly what the previous commit's terminal-failure approach could not detect.
The three existing tests move to the unified accessor.
Also consolidates the [Unreleased] changelog section, which had accumulated
duplicate Added/Changed/Fixed headings across this branch's commits, and corrects
the entry that described the fix in terms of a producer dying.
Verified: 84 tests across AbstractProducerTest, TuneConfigurationTest,
ProducerStateTest, ProducerJavaTest, ProducerOpenCLTest and FinderTest.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
An audit of what this branch learned versus what it can prove found three gaps.
Everything asserted in Java about the reporting model was covered; what a user
actually reads was not, and nothing in the repository exercised the Python at all
-- which is where one of the bugs actually was.
1. Report rendering. describeArm/buildReport had no assertions whatsoever, and
the report was changed twice on this branch. Now pinned per outcome: a
measured arm shows its rate and no marker, a rejected arm says FAILED and
carries the driver's cause, and a too-slow arm says NOT MEASURED, states it is
not a device error, and names the lever to pull. Two situations printing alike
is what made the Arc investigation slow.
2. MultilineLogger integration. MultilineLoggerTest proved the helper; nothing
proved the call sites use it. Reverting TuneConfiguration or Main to a single
LOGGER.info would have left every test green while users got the unreadable
' | ' form back. Both are now asserted end to end, including that no emitted
record contains a line break -- the property the CRLF guard exists to enforce.
3. Python had no tests and no way to run any. Added stdlib-unittest coverage for
register_machine.py: the nvidia-smi short-circuit, the list schema, --set
comma splitting, the empty-list-preserves-existing path, machine-id
determinism, and a check that every shipped registry entry matches the schema.
Stdlib rather than pytest on purpose -- the script is meant to be downloaded
as a single file and run by a contributor, so its tests should not demand an
install either.
Run with: python -m unittest discover -s docs/measurements -p 'test_*.py'
Writing them immediately found two real defects, both fixed here:
- The two-directional dedup in detect_gpu was one-directional. It skipped a new
name contained in a kept one but not the reverse, so "NVIDIA RTX 500 Ada"
from nvidia-smi followed by "NVIDIA RTX 500 Ada Generation Laptop GPU" from
the OS recorded the same card twice. The more specific name now wins. This
was in the fix committed earlier on this branch and hand-verification missed
it, because the machine it was verified on reports identical names.
- main() crashed with ValueError on its own success message when the registry
sits outside the repository, because relative_to() raises rather than
degrading. It now falls back to the absolute path.
14 Python tests (0.1 s, hardware probes stubbed for the registry cases) and 84
Java tests across TuneConfigurationTest, MainTest, MultilineLoggerTest,
AbstractProducerTest, StatisticsTest and RuntimeStatisticsTest. REUSE still
486/486.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…take Running the new register_machine tests produces docs/measurements/__pycache__, and the previous commit swept it in with git add -A. Two generated .pyc files therefore landed in the repository, and REUSE dropped to 487/489 because binary bytecode carries no licence header and never should. .gitignore had no Python rules at all, since until now nothing in the repository was executed as Python by a test. Adding __pycache__/ and *.py[cod] stops this recurring for the next person who runs them. Verified: REUSE back to 487/487 compliant. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A winner sitting on the largest value swept is a lower bound, not a peak: the
sweep stopped looking, not the hardware. The previous answer to that was a
sentence in the guide telling the operator to widen the list and start over,
which only reaches someone who reads it beforehand — and a 20-minute measurement
is run once. Both of a contributor's first sweeps in discussion 281 won at the
shipped list's last keysPerWorkItem of 256; widening by hand afterwards gained
17 % on their RTX 500 and 104 % on their Arc.
TuneConfiguration now continues past the configured candidates on its own,
doubling keysPerWorkItem or adding one bit to batchSizeInBits, stopping at the
first extra arm that fails to beat the winner. The usual cost of being wrong is
therefore one arm, not a second sweep. maxExtensionArms (default 4) bounds the
case where every step keeps improving, and extendSweepWhenWinnerIsAtTheEdge
turns it off.
Edge of the list versus edge of the possible
--------------------------------------------
Conflating the two would make this useless, so they are separated explicitly:
keysPerWorkItem bounded by 2^batchSizeInBits, because OpenClTask computes work
items as grid / keysPerWorkItem and rejects anything larger
batchSizeInBits bounded by BIT_COUNT_FOR_MAX_CHUNKS_ARRAY (24), which the
shipped candidate list already reaches
A winner at 24 is therefore at the framework maximum, not at a truncation.
Extending there would only produce a rejected arm, and flagging it would fire on
most default runs until nobody read it any more. The report states the
distinction in words via winnerBoundaryNote, so a reader seeing the winner in the
table's last row knows which of the two happened.
A CPU producer is excluded outright: keysPerWorkItem is inert there, so extending
would spend minutes re-measuring one arm.
Written test-first; all nine assertions were confirmed red before the
implementation existed. They pin both boundaries, the CPU case, a failed winner,
and the report wording. One further test proves the loop actually runs on real
hardware — a one-candidate sweep necessarily wins at the edge, so leaving
extension on must produce more arms than candidates, which the decision-function
tests cannot show because they never start a producer.
run_openClProducer_producesOneArmPerCandidatePair now switches extension off. It
asserts the grid enumeration alone, and its expected count is a cross product;
the failure it produced here was the new behaviour working, not a regression.
Architecture rule: "Command" is added to the Constants layer's accessor list.
Reading BIT_COUNT_FOR_MAX_CHUNKS_ARRAY is the point — hard-coding 24 in the
command would be a second source of truth for a ceiling the kernel layout owns.
Constants is the leaf, so the dependency is downward and adds no cycle.
Also adds .github/workflows/measurement-tools.yml, a separate pipeline for the
Python tooling tests: no JDK, no Maven, sub-second, path-filtered to
docs/measurements. Kept out of publish.yml because it shares none of that setup,
and out of every unrelated push because a check that is usually irrelevant is a
check people learn to ignore. Python 3.9 and 3.13 are both exercised; 3.9
compatibility was verified with ast.parse(feature_version=(3, 9)) rather than
assumed.
The guide's "widen and re-run" advice is replaced by a description of what the
tool now does by itself.
Verified: 114 Java tests, 14 Python tests, REUSE 487/487, spotless clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The tuner took producerOpenCL.get(0) and ignored the rest without a word. Someone
with two GPUs naturally lists both, waits twenty minutes, and receives a
full-looking report for one device plus a paste-ready configuration whose second
entry still holds their guesses -- rendered identically to the measured one. On
the contributor's laptop in discussion 281 that guess was 21/256 for a device
whose measured optimum was 23/2048: 16.0 M keys/s where 36.5 M were available,
2.3x off, with nothing in the output able to reveal it. That is the same class of
quietly-wrong result this branch has been removing throughout.
All configured producers are now swept in turn, each keeping its own winner,
which is applied to its own entry. Picking one winner across devices would have
handed the integrated GPU the discrete card's 24/2048.
An empty producerOpenCL list used to be an error. It is instead the state most
first runs are in -- the operator does not yet know what to write -- so it now
means "sweep every GPU you find". Being told to configure a producer first is no
use to someone who ran the tuner precisely to learn what to configure. Listing
entries explicitly remains the override, and when the machine has more GPUs than
were listed the log says so rather than leaving it to be noticed.
Details that make it usable rather than merely correct:
- CPU OpenCL runtimes are excluded. pocl and Intel's CPU ICD enumerate as
devices but duplicate producerJava while costing a full sweep of wall-clock,
and the project's own OpenCL CI job runs on pocl.
- Devices are measured one at a time. Two at once contend for host memory
bandwidth: this session measured an integrated GPU losing 3x its solo
throughput that way, so sweeping in parallel would measure the interference.
- The filter payload is built once and reused across devices. Only the sweep
repeats, so N devices cost N sweeps plus one build, and the run time is
stated up front.
- Templates are copied by JSON round-trip, not field assignment. CProducerOpenCL
has fifteen-odd settings and gains more; a hand-written copy silently drops
whichever was added last, and the result would be a device measured under
settings nobody chose.
- The emitted configuration gives each producer its own key producer. Sharing
one is accepted at runtime -- KeyProducerIdIsNotUniqueException guards
duplicate definitions, not shared references -- but every shipped
multi-producer example gives each its own, and a generated config should not
ship a convention violation the operator never made.
The example config now ships with an empty producerOpenCL, so the copy-paste path
gets the all-devices behaviour rather than a hard-coded platformIndex 0.
ConfigFixturesParseTest asserts that emptiness deliberately.
Architecture rule: "Command" is added to the Opencl layer's accessor list. The
tuner has to enumerate devices to sweep them, which is the discovery Cli already
performs for OpenCLInfo; the alternative is passing a device list into a command
built from configuration alone.
Also refreshes the database tier figures: targetDatabaseEntries defaults to
141045995 (Light, 2026-07-20 publication) instead of the stale 132288304, and the
Full tier reference becomes 1472947953. Historical measurement tables keep the
counts they were actually measured at -- changing those would falsify records
rather than update them.
Written test-first. Verified: 120 Java tests, 14 Python tests, REUSE 487/487,
spotless clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…een via two ICDs On a machine where two OpenCL platforms each expose the same physical GPUs (a duplicate ICD registration, common on Windows AMD systems), the auto-sweep counted every platform*device entry as a distinct GPU: four entries for two cards. It then measured each card twice and emitted a producerOpenCL list that drove each card with two contending producers, and the "N GPUs are present" hint over-reported the count. - Add OpenCLDeviceTopology to resolve a PCIe bus/device/function fingerprint via AMD's cl_amd_device_attribute_query (CL_DEVICE_TOPOLOGY_AMD). Any other vendor, a missing extension, or a driver error yields null, which is treated as "unknown, keep the device" -- so an unsupported device can only fall back to today's behaviour, never regress it, and a rig of identical cards (same name, distinct PCIe slots) is never wrongly merged. - dedupePhysicalDevices collapses entries that share a non-null fingerprint, keeping the first. gpuDevicesOf now returns physically-distinct GPUs, so both the sweep and the undetected-devices hint see the real count. - renderPerDeviceWinners adds a "Winner per device (MEASURED)" section to the report. The per-device optimum was previously only visible in the emitted JSON; the human report showed a single global winner over a device-unlabelled arms table, hiding that a slower card wanted different settings. Validated on a Radeon RX 7900 XTX + Radeon iGPU box with a duplicate AMD ICD: four detected entries collapse to two by PCIe fingerprint, and the report lists each card's own winner. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015g913gVNXb5wJfnZMM6E96
…no GPU filter is needed initLMDB opened the LMDB env unconditionally, so disableAddressLookup=true still required a valid lmdbDirectory on disk: with none, Find failed at startup with an lmdbjava ESRCH error (or, without the JVM add-opens flags, an InaccessibleObjectException). disableAddressLookup only ever short-circuited the per-key containsAddress call, never the env open, so the documented "run without a database" fallback did not actually work. When address lookup is disabled and no GPU pre-filter has to be built from the database, skip the env open entirely and answer every lookup as absent via a self-contained AbsentAddressPresence. The GPU-filter path is unaffected: it still needs LMDB open and is explicitly excluded from the skip. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015g913gVNXb5wJfnZMM6E96
… runs unaided The runnable fat jar carried only Main-Class, so `java -jar bitcoinaddressfinder-*-jar-with-dependencies.jar <config>` crashed the moment it opened LMDB: lmdbjava reflects into java.nio.Buffer.address / sun.nio.ch / jdk.internal.ref, which Java 21 does not open by default. The examples/run_* launchers pass the flags on the command line, but a bare `java -jar` did not, and nothing in the output pointed at the cause. Bake the java.base add-opens set (java.lang, java.io, java.nio, jdk.internal.ref, jdk.internal.misc, sun.nio.ch) into the assembly launcher manifest so the fat jar is self-sufficient. Kept in sync with .mvn/jvm.config; the wider jdk.compiler/jdk.management opens the run scripts also carry are for Error Prone and JMH, which the fat jar never runs. Validated: `java -jar` (no flags) now runs Find against a real 6.2 GB LMDB. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015g913gVNXb5wJfnZMM6E96
…on, guard it Every examples/run_*.bat|sh launcher and docs/tuning-your-gpu.md still named bitcoinaddressfinder-1.7.0-jar-with-dependencies.jar, but a build from source produces 1.8.0-SNAPSHOT, so the launchers failed with "jar not found" after the version bump. Update all 22 launchers and the guide to the current version. Add ExampleRunScriptJarVersionTest as a hard gate: it reads the project version from pom.xml and asserts every bitcoinaddressfinder-<v>-jar-with-dependencies.jar reference in the launchers and docs matches, so the next version bump cannot leave them stale. The jar-name pattern never matches a plain version mention, so scanning docs is safe. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015g913gVNXb5wJfnZMM6E96
…, and correct the docs Follow-up to the four commits pulled from the dual-GPU machine. The work was sound; three things needed finishing. 1. The de-duplication was AMD-only, and it did not have to be. It keyed on cl_amd_device_attribute_query, with a null fallback for every other vendor -- correct but narrow: a non-AMD machine got no fingerprint at all, so a card seen through two ICDs would still have been swept twice. A contributor's device dump settles it: both an NVIDIA RTX 500 Ada and an Intel Arc report cl_khr_device_uuid and cl_khr_pci_bus_info, which are Khronos standards designed for exactly this. The resolution order is now UUID, then PCI bus info, then the AMD vendor extension for drivers predating both. The method is renamed accordingly: pciFingerprintOf -> physicalDeviceFingerprintOf, since a UUID is not a PCIe address and the old name would have lied about two of the three sources. The parsers are separated from the CL call and unit-tested, which the AMD path never was -- it could only be exercised on the one machine that had the hardware. That matters more than usual here: a wrong offset yields a plausible-looking fingerprint, and a plausible-looking wrong fingerprint either merges two distinct GPUs (silently halving a rig) or fails to merge one card seen twice. Neither surfaces as an error at runtime. 14 tests now pin the byte layouts, the all-zero-UUID rejection, unsigned bus numbers, host byte order, and whole-token extension matching. Two defects found while writing them: the AMD type field was read as its first byte rather than as the cl_uint it is (correct only on little-endian hosts), and extension detection used a substring match, which would claim cl_khr_device_uuid support on a device advertising only cl_khr_device_uuid_extended. 2. The manifest change made four documentation passages wrong. Baking Add-Opens into the fat jar means a bare `java -jar` now works, but the README and the tuning guide still said it throws InaccessibleObjectException, and the guide's Troubleshooting entry diagnosed a problem that can no longer occur -- the worst kind of stale documentation, since it sends a reader after a cause that is not there. All four corrected, keeping the honest note that older jars do need the flags, and that the launchers remain useful for the heap and Logback settings they also set. 3. The pom comment asked for a sync that nothing enforced. It said to keep the manifest's Add-Opens in step with .mvn/jvm.config and named JvmModuleFlagConsistencyTest as the canonical list -- but that test never read the manifest. It does now: the entry must equal the java.base opens from the master set exactly. A package dropped from it would otherwise fail only at runtime, on a user's machine, when LMDB is opened. Also adds the CHANGELOG entries the four pulled commits deferred: the ICD de-duplication, the self-sufficient fat jar, the disableAddressLookup fix, and the jar-version guard. Deliberately not done: an NVIDIA-specific CL_DEVICE_PCI_BUS_ID_NV path. It is now unnecessary -- NVIDIA reports the Khronos extensions, which the generic path already uses. Verified: 126 tests across JvmModuleFlagConsistencyTest, OpenCLDeviceTopologyTest, TuneConfigurationTest, ExampleRunScriptJarVersionTest, ConsumerJavaTest, MainTest and the architecture rules. REUSE 487/487, spotless clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
… it PCIe
Two loose ends from making the fingerprint vendor-neutral.
The skip message said "PCIe {}" while printing a value that is now a
cl_khr_device_uuid on most hardware. It was accurate when the only source was
AMD's topology extension and is not any more.
More useful: the identity was only visible when a merge happened. With no merge
the log could not distinguish "no duplicate is present" from "no fingerprint
could be read at all" — and those call for opposite conclusions when someone is
checking whether de-duplication works on their machine. Every detected device now
reports its identity, or "unknown".
Verified: 54 tests across TuneConfigurationTest and OpenCLDeviceTopologyTest.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
De-duplication merged on a shared fingerprint alone. The failure modes are not
symmetric: a missing fingerprint costs nothing (both entries kept, which is what
happened before this feature existed), while a fingerprint that is wrongly
identical merges two distinct GPUs and silently halves a multi-GPU machine, with
nothing in the run reporting it.
That is not hypothetical exposure. Of the three fingerprint sources, only AMD's
topology extension has ever executed on real hardware — the one dual-ICD machine
available reports neither cl_khr_device_uuid nor cl_khr_pci_bus_info, so both
Khronos paths are unverified outside unit tests. A driver handing every device
the same UUID is exactly the defect that would not be noticed.
Merging now also requires the device name to match. The name stays a veto and
never becomes a merge signal — merging on names would still be wrong, since a rig
of identical cards shares one. The legitimate case is unaffected: one card seen
through two ICDs reports the same name, verified as gfx1100 on both platforms of
an AMD box. Should two ICDs ever name one card differently, the merge is skipped
and both are swept: a missed optimisation, not a loss.
Written test-first; the guard test was confirmed red against the previous
behaviour.
Also, unrelated to the above but overdue:
- CLAUDE.md instructed ./mvnw seven times. There is no wrapper in this
repository, so every one of those commands fails when followed literally. I
hit this myself earlier in this branch's work. Switched to mvn, with the
absence stated explicitly rather than left to be rediscovered.
- The README's "Keys per producer" description now says to read it alongside
"Producer blocked (queue full)": while that counter rises the consumer is the
bottleneck, so the shares describe what the queue allowed each producer to
deliver, not what the devices can do. Observed during the dual-GPU
verification and easy to misread as a device comparison.
Verified: 81 tests across TuneConfigurationTest, OpenCLDeviceTopologyTest,
JvmModuleFlagConsistencyTest, ExampleRunScriptJarVersionTest,
ConfigFixturesParseTest and the architecture rules. REUSE 487/487, spotless clean.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…he global winner Found by counter-testing on a dual-GPU machine, and it was my bug: multi-device sweeping was added on top of the extension logic without revisiting which winner the extension consults. extendSweepBeyondTheCandidates called pickWinner over the global results accumulator. On a two-device run the integrated GPU won at the largest keysPerWorkItem it was given and should have been extended, but was not — the global winner was the discrete card's arm, comfortably inside its own range, so the decision reported "nothing to extend". The slower device's reported optimum is then a silent lower bound, which is the precise defect the extension exists to remove. A single-GPU machine cannot show this: there the global winner and the device winner are the same arm. It took hardware with two devices of very different speed. The extension now receives the device's own arms. nextExtensionArmForDevice picks the winner from that slice, so consulting another device's result is structurally impossible rather than merely corrected. Written test-first; the new test was confirmed red, and its counterpart pins that a device winning inside its range still must not be extended. Also qualifies the interference figure in docs/tuning-your-gpu.md §8. It cited "~3×" from a single laptop pair. The same measurement on a desktop pair (AMD gfx1036 beside an RX 7900 XTX) gives ~1.5%: 3.12 M keys/s solo against 2.07 M/s alongside. Both are now shown together with the advice to measure your own pair — one sample was never enough to state a ratio as fact. Not acted on: the reported OOM on shipped defaults. The diagnosis that came with it — "host result buffer scales as 2^batchSizeInBits x keysPerWorkItem, so batch24/kpwi256 needs ~274 GB" — does not match the code. OpenClTask.dstSizeInBytes is 4 + 108 x 2^batchSizeInBits and does not involve keysPerWorkItem at all: 0.42 GiB at batch22, 1.69 GiB at batch24. The OOM is real and reproducible, but its cause is not established, and capping grid sizes against a formula that is off by a factor of keysPerWorkItem would cripple the tuner for a phantom. Asked for the stack trace and the exact config instead. Verified: 79 tests across TuneConfigurationTest, OpenCLDeviceTopologyTest, ConfigFixturesParseTest and the architecture rules. REUSE 487/487, spotless clean. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The OOM the dual-GPU machine hit is mine, and the cause is not what its first
diagnosis said. The reported model — host buffer scaling as 2^batchSizeInBits x
keysPerWorkItem, ~274 GB at the largest shipped corner — does not match
OpenClTask.dstSizeInBytes, which is 4 + 108 x 2^batchSizeInBits, off-heap, and
1.69 GiB at batch24. Acting on that model would have capped grid sizes against a
phantom. The follow-up stack trace settled it:
OutOfMemoryError: Java heap space
at OpenCLGridResult.readEntry(OpenCLGridResult.java:227)
at ProducerOpenCL$ResultReaderRunnable.run(ProducerOpenCL.java:334)
With no GPU pre-filter every candidate is read back and enqueued, so the reader
materialises a full grid of PublicKeyBytes per queued batch and per reader thread.
It is a host-heap limit and it scales with 2^batchSizeInBits alone —
keysPerWorkItem is irrelevant, exactly as the code says.
Why the tuner walked into it: auto-discovered devices were built from
new CProducerOpenCL(), whose enableGpuFilter defaults to false. That default was
wrong here in three separate ways. The tuner builds a filter payload for
targetDatabaseEntries regardless — about a minute at 141 M entries — and then
never used it. It measured full-transfer throughput while claiming to describe the
pipeline the operator will run, which both the guide and the emitted
recommendation put behind a filter. And it transferred every candidate, which is
what exhausted the heap.
Emptying the shipped example's producerOpenCL list to demonstrate auto-discovery
is what moved it onto those defaults; the example had carried
enableGpuFilter: true until then. So the regression arrived with the change that
was meant to make the example simpler.
Auto-discovery now sets enableGpuFilter explicitly. A sweep that still runs
without one warns before starting, with the estimated host-heap cost against the
actual maximum heap — the JVM's own OutOfMemoryError names neither the setting nor
the grid size responsible, which is precisely why this was first read as a
device-memory problem. The threshold is deliberately eager (a quarter of the heap)
and the per-candidate estimate coarse: a sweep that dies twenty minutes in costs
more than a warning nobody needed.
Test fixture: the tuner tests now pin targetDatabaseEntries to 1000. With the
filter on, the 141 M default would have every OpenCL test build a genuine Binary
Fuse filter inside a 2 GiB Surefire fork — which is how this change first
announced itself, as an OOM in the test fork rather than in the code under test.
Also ignores *.hprof. pom.xml sets -XX:+HeapDumpOnOutOfMemoryError, so reproducing
this dropped multi-GB dumps into the working tree and nothing kept them out.
Verified: 133 tests across TuneConfigurationTest, ConsumerJavaTest, MainTest,
OpenCLDeviceTopologyTest, ConfigFixturesParseTest and the architecture rules.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…peline Full shipped-default sweep (7x5 candidates up to batch24/kpwi256, secondsPerArm=20, real targetDatabaseEntries=140964881) on 9fbe4bf, run with a bare `java -jar` and no -Xmx override -- the exact scenario that previously OOM'd at arm 22/35 now completes cleanly, because auto-discovered devices sweep with the GPU pre-filter enabled. One row per arm, including the per-device sweep extensions. Per-device winners: gfx1100 (RX 7900 XTX) at batch24/kpwi512 ~190 M/s, gfx1036 (iGPU) at batch23/kpwi256 ~4.6 M/s; both winners sat on the kpwi edge and were extended past the candidate list, each judged against its own device (not the global winner). NOTE: these figures measure the FILTERED pipeline -- only FUSE_8 survivors are read back to the host, which is what the tuner's recommendation actually describes. Earlier numbers on this machine measured full candidate transfer and are NOT comparable. Verification cost 6.60 us measured against the real 141M-entry database; out-of-line (AMD_NOINLINE_HELPERS) kernel; machines.json already current (dry-run matches). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015g913gVNXb5wJfnZMM6E96
ryzen79800x3d8core-61g-win11 now has two tuner files, and their throughput figures differ by an order of magnitude for the same card: the older one measured a single device through the full-transfer path, the new one measures both GPUs through the GPU-filtered path where only filter survivors cross back to the host. That is a different pipeline, not a better result, and nothing in the directory said so. The older file stays. This directory's own rule is that stale rows are kept for provenance rather than deleted, and the reviewer who raised the question was right not to remove it unasked. Also records what the file table left implicit: in a multi-device sweep the winner flag is per device, so such a file carries one "yes" per GPU rather than one for the file. Verified against the new data — each flagged row is in fact its own device's maximum. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
bernardladenthin
had a problem deploying
to
maven-central
July 26, 2026 20:24 — with 
GitHub Actions
Failure
GitHub Actions
Failure
bernardladenthin
had a problem deploying
to
maven-central
July 26, 2026 20:24 — with
GitHub Actions
Failure
GitHub Actions
Failure
bernardladenthin
had a problem deploying
to
startgate
July 26, 2026 20:24 — with
GitHub Actions
Error
GitHub Actions
Error
8 tasks
bernardladenthin
added a commit
that referenced
this pull request
Jul 27, 2026
…pocl-gpu-assume Fix the CI failures from #320: SpotBugs findings and a GPU-only assumption on pocl
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.
Summary
TuneConfigurationused to takeproducerOpenCL.get(0)silently: a dual-GPU config produced a full-looking report for one card plus a paste-ready configuration whose second entry still held the operator's guesses, rendered identically to the measured one. All configured devices are now swept in turn, each keeping its own winner, and an emptyproducerOpenCL— previously an error — means "sweep every GPU you find", which is the state most first runs are in.0.00arm was indistinguishable from a driver rejection, batch counts read as an even split where the real work split was 20:1, and every rate above 1 G/s collapsed to1 G/s. Each of those made a bad number look like a good one.Driven by field reports in #250 and discussion #281, and counter-tested throughout on a second machine with two GPUs behind a duplicate ICD. That machine found three defects unreachable from single-GPU hardware — all fixed here.
Behaviour changes a reviewer should know about
Batches per producer→Keys per producer, with each device's share2^batchSizeInBitscandidatesproducerOpenCLnow means "sweep all detected GPUs" (was an error)disableAddressLookup: trueno longer opens LMDBjava -jarAdd-Opensmoved into the assembly manifest; launchers still set heap and Logback1.000and1.499 G/sboth printed as1 G/sCli→Util,Command→Constants,Command→OpenclTest plan
TuneConfigurationTest,ConsumerJavaTest,MainTest,AbstractProducerTest,StatisticsTest,RuntimeStatisticsTest,MultilineLoggerTest,OpenCLDeviceTopologyTest,JvmModuleFlagConsistencyTest,ExampleRunScriptJarVersionTest,ConfigFixturesParseTestand the architecture rules, plus 14 Python testsWritten test-first throughout; every fix here had a failing test before it had an implementation. A full
mvn testwas not run — per the repo's test-scoping policy, only the affected surface was exercised. Verification on real hardware (dual-GPU, dual-ICD, LMDB, barejava -jar) happened on a second machine and is recorded in the commit messages.Known limitations, deliberately left
cl_khr_device_uuidandcl_khr_pci_bus_infofingerprint paths have never executed on real hardware. The only dual-ICD machine available reports neither extension, so only the AMD fallback ran there. They are unit-tested against their byte layouts, and de-duplication additionally requires the device name to agree — so the dangerous failure (a driver reporting one identity for several devices, silently halving a machine) cannot occur even if a path is wrong.CLAUDE.mdreferenced./mvnwseven times and no wrapper exists — corrected tomvnhere rather than adding a wrapper, which is a separate decision.Related issues / PRs
Refs #250, refs #281
Checklist
CONTRIBUTING.mdandCODE_OF_CONDUCT.mdutil/MultilineLoggerand covered by tests.