The memory search has two passes, and remembers where a card's memory gave out

README.md

@@ -176,7 +176,7 @@ flowchart LR
 
 - **Every run starts with the card at stock for five minutes,** as long as the result is tested at the end, with the fans left to the NVIDIA driver. That is the "before" of the result: a card is not heated through after 30 s, so a shorter run would make every result look warmer and louder than stock. Some cards get hot there: the driver's own fan control lets them (a 3080 Ti reached 85 °C with its fans at 65 %). The measurement stops at 85 °C, what it measured until then is the "before", and the run goes on. From there every step keeps the fans on a curve that reaches 100 % at the profile's temperature limit.
 - **Stress test.** A DirectX 11 compute load in which the GPU checks every value it computes against a known answer. Each probe ends in a verdict: `STABLE`, `WRONG RESULT`, `DEVICE LOST`, `STALLED` (the values were right but the card computed almost nothing), `TOO HOT` or `NO TELEMETRY`.
-- **Memory comes first and stops at the bandwidth peak,** not at the first error. GDDR6 and GDDR6X retry failed transfers, so an overclocked memory bus loses speed long before it returns a wrong result. The search raises the offset 50 MHz at a time, measures bandwidth at each step and keeps the lowest offset within 1 % of the best. Where bandwidth cannot be measured, the memory is left at stock: without that measurement nothing says where to stop, and the first wrong result comes far too late.
+- **Memory comes first and stops at the bandwidth peak,** not at the first error. GDDR6 and GDDR6X retry failed transfers, so an overclocked memory bus loses speed long before it returns a wrong result. The search raises the offset 50 MHz at a time, measures bandwidth at each step until a step fails or has clearly lost bandwidth, then measures again from 200 MHz below that point down and keeps the lowest offset within 1 % of the best. Where the memory gave out is remembered for that card and driver, so a later run stays below it without going there again. Where bandwidth cannot be measured, the memory is left at stock: without that measurement nothing says where to stop, and the first wrong result comes far too late.
 - **The core search climbs from stock.** With the chosen memory offset applied, it raises the core offset 15 MHz at a time until a probe fails, and keeps the last value that passed. How far it may go comes from the range the driver reports for your card, so a card that can hold more is not stopped at a fixed limit.
 - **Each edge is confirmed.** The value a search ends on gets a 30 s probe, and a lower value is tried if it does not hold. The safety margin is applied to the confirmed value, and the result as a whole must pass the 300 s soak.
 - **A driver reset ends that clock's search.** On a card the search can push past its edge, a search resets the driver: the screen goes black for a moment. The run then reconnects to the driver, sets the card to stock, leaves it without load for 20 seconds and checks with a short probe at stock that it computes normally again. After that the search tries no new values for that clock: it keeps the last value that passed and confirms four steps below it. Each clock has its own search, so a reset in the memory search does not keep the core from being searched; a run can therefore reset the driver twice, once per clock. One reset more than that ends the run at stock with nothing saved, and so does a card that does not come back. Results on real cards are in [docs/hardware-checks.md](docs/hardware-checks.md), checks 51 and 52.

src/app/common.cpp

@@ -355,7 +355,10 @@ OptimizeOutcome run_optimize(Preset preset, const OptimizeHooks& hooks, const st
     if (path.empty()) return fail("the ProgramData folder could not be resolved; cannot keep the crash journal");
     const auto lines = read_lines(path);
     if (!lines) return fail("the crash journal " + path.string() + " exists but cannot be read; not tuning without it");
-    Journal journal(*lines, [&path](const std::string& l) { return append_line_durable(path, l); });
+    // What a memory edge in the journal is bound to: this card with this driver.
+    const std::string rig_card = hw.GpuUuid(), rig_driver = hw.DriverVersion();
+    Journal journal(*lines, [&path](const std::string& l) { return append_line_durable(path, l); },
+                    rig_card.empty() || rig_driver.empty() ? std::string() : rig_card + " / " + rig_driver);
     // Prove the journal is writable before any clock is touched; the parser
     // ignores lines without an id, so this one never becomes a ceiling.
     if (!append_line_durable(path, "{\"session\":\"" + now_text() + "\"}"))

src/core/journal.cpp

@@ -43,8 +43,8 @@ std::optional<int> int_field(const nlohmann::json& j, const char* key) {
 }
 }
 
-Journal::Journal(const std::vector<std::string>& existing_lines, Append append)
-    : append_(std::move(append)) {
+Journal::Journal(const std::vector<std::string>& existing_lines, Append append, std::string rig)
+    : append_(std::move(append)), rig_(std::move(rig)) {
     std::map<int, Pending> pending;   // ordered by id = file order
     int max_id = 0;
     for (std::string line : existing_lines) {
@@ -53,6 +53,14 @@ Journal::Journal(const std::vector<std::string>& existing_lines, Append append)
         const bool parsed = !j.is_discarded() && j.is_object();
         torn_tail_ = !line.empty() && !parsed;   // only the last line's value survives the loop
         if (!parsed) continue;
+        if (const auto edge = j.find("mem_edge"); edge != j.end()) {
+            // A line of its own, without an id: builds that do not know it skip it.
+            const auto mhz = int_field(j, "mem_edge");
+            const auto rig_of = j.find("rig");
+            if (mhz && *mhz > 0 && *mhz <= kMaxClockMhz && !rig_.empty() && rig_of != j.end() && rig_of->is_string() && *rig_of == rig_)
+                mem_edge_ = std::min(mem_edge_, *mhz);
+            continue;
+        }
         const auto id = int_field(j, "id");
         const auto state = j.find("state");
         if (!id || *id <= 0 || *id >= kMaxJournalId || state == j.end() || !state->is_string()) continue;
@@ -92,6 +100,14 @@ bool Journal::complete(int id, const std::string& verdict) {
     return true;
 }
 
+bool Journal::note_mem_edge(int mhz) {
+    if (mhz <= 0 || rig_.empty()) return true;   // nothing to bind it to: nothing is kept
+    const nlohmann::json j = {{"mem_edge", mhz}, {"rig", rig_}};
+    if (!write(j.dump())) return false;
+    mem_edge_ = std::min(mem_edge_, mhz);
+    return true;
+}
+
 bool Journal::write(const std::string& line) {
     if (!append_(torn_tail_ ? "\n" + line : line)) return false;
     torn_tail_ = false;

src/core/journal.hpp

@@ -30,11 +30,18 @@ struct Freeze {
 // that froze the machine, and it becomes a ceiling for every later run. An
 // offset of 0 is never a ceiling: that would end the search of that clock.
 // Pure: the file lives in hw/app_files, reached through `append`.
+//
+// The journal also remembers where the memory of a card gave out in a scan
+// that finished (a wrong result, a driver reset, a lost device): a card whose
+// bandwidth rises until its memory fails has nothing else to stop a scan, and
+// every run would meet that edge again. Such an edge belongs to a card with a
+// driver, `rig`; with another of either it does not count. An empty rig keeps
+// and uses none.
 class Journal {
 public:
     using Append = std::function<bool(const std::string&)>;
 
-    Journal(const std::vector<std::string>& existing_lines, Append append);
+    Journal(const std::vector<std::string>& existing_lines, Append append, std::string rig = {});
 
     const Ceilings& ceilings() const { return ceilings_; }
     // Human-readable description of each unfinished candidate, in file order:
@@ -43,6 +50,12 @@ class Journal {
     // The same candidates, each with whether it lowered a ceiling.
     const std::vector<Freeze>& freeze_entries() const { return freeze_entries_; }
     int next_id() const { return next_id_; }
+    // The lowest memory offset at which a scan on this rig gave out; INT_MAX
+    // when none is known. Not a ceiling: the search decides how far below it
+    // to stay.
+    int mem_edge() const { return mem_edge_; }
+    // Writes one down. False when the line could not be written.
+    bool note_mem_edge(int mhz);
     // The entry begin() opened and complete() has not closed; -1 when none.
     int open_id() const { return open_id_; }
 
@@ -60,6 +73,8 @@ class Journal {
     // by a power cut, without its newline. The next write starts with '\n'
     // so it is not glued onto that fragment.
     bool torn_tail_ = false;
+    std::string rig_;
+    int mem_edge_ = INT_MAX;
     Ceilings ceilings_;
     std::vector<std::string> freezes_;
     std::vector<Freeze> freeze_entries_;

src/core/search.cpp

@@ -61,26 +61,59 @@ int apply_margin(int max, int step, float perf_push) {
     return std::max(0, std::min(steps * step, max - step));
 }
 
-int best_bandwidth_offset(int lo, int hi, int step, int ceiling, const std::function<MemSample(int)>& sample) {
-    const int top = std::min(hi, ceiling - 1);
-    std::vector<std::pair<int, double>> seen;
+MemScanEnd bandwidth_scan(int lo, int hi, int step, int ceiling, const std::function<MemSample(int)>& sample) {
+    MemScanEnd end{lo, MemScanEnd::Why::Limit};
+    auto middle = [](std::vector<double> of) {   // of an even number: the lower one
+        std::sort(of.begin(), of.end());
+        return of[(of.size() - 1) / 2];
+    };
+    std::vector<double> steps;   // the reading of every step that passed
     double best = 0;
-    for (int v = lo; v <= top; v += step) {
+    for (int v = lo; v <= std::min(hi, ceiling - 1); v += step) {
         MemSample s = sample(v);
-        if (!s.stable) break;
-        // A drop is measured twice before it ends the scan: a single reading
-        // can land while the memory clock is still in a lower P-state.
-        if (s.gbps < best * (1 - kBandwidthDrop)) {
-            s = sample(v);
-            if (!s.stable) break;
+        // A reading that would end the scan is in doubt: the step is read
+        // kBandwidthReadings times and the middle one counts. Only such a
+        // step: three readings at every step cost two minutes a scan and
+        // decided nothing a single reading did not (hardware check 95).
+        if (s.stable && s.gbps < best * (1 - kBandwidthDrop)) {
+            std::vector<double> readings{s.gbps};
+            while (static_cast<int>(readings.size()) < kBandwidthReadings && (s = sample(v)).stable) readings.push_back(s.gbps);
+            s.gbps = middle(readings);
         }
+        if (!s.stable) { end.why = MemScanEnd::Why::Failed; break; }
+        end.top = v;
+        if (s.gbps < best * (1 - kBandwidthDrop)) { end.why = MemScanEnd::Why::Dropped; break; }
+        // The best is the highest middle of three steps in a row, not the
+        // highest reading: one reading that came out high would be a best
+        // every later step is held against, and the highest of fifty readings
+        // that scatter is always such a one.
+        steps.push_back(s.gbps);
+        best = std::max(best, middle({steps.end() - static_cast<std::ptrdiff_t>(std::min<size_t>(steps.size(), 3)), steps.end()}));
+    }
+    return end;
+}
+
+int second_pass_top(const MemScanEnd& end, int lo, int step, int known_edge) {
+    // The lowest offset that is known, or has to be assumed, not to hold.
+    int bound = end.top + step;
+    if (end.why == MemScanEnd::Why::Limit && known_edge != INT_MAX && known_edge > bound) bound = known_edge;
+    return std::max(lo, bound - step - kPeakMarginSteps * step);
+}
+
+int bandwidth_peak_below(int top, int lo, int step, const std::function<MemSample(int)>& sample) {
+    std::vector<std::pair<int, double>> seen;
+    double best = 0;
+    for (int v = top; v >= lo; v -= step) {
+        const MemSample s = sample(v);
+        if (!s.stable) break;
         seen.emplace_back(v, s.gbps);
         best = std::max(best, s.gbps);
-        if (s.gbps < best * (1 - kBandwidthDrop)) break;
+        if (s.gbps < best * (1 - kBandwidthTie)) break;
     }
+    int peak = top;
     for (const auto& [v, gbps] : seen)
-        if (gbps >= best * (1 - kBandwidthTie)) return v;
-    return lo;
+        if (gbps >= best * (1 - kBandwidthTie)) peak = std::min(peak, v);
+    return peak;
 }
 
 int confirm_edge(int edge, int lo, int step, int tries, const std::function<bool(int)>& holds,
@@ -580,25 +613,49 @@ OptimizeResult optimize(const GpuControl& gpu, const Objectives& obj, Journal& j
     // Memory comes first, at core 0: the core climb is where a card is most
     // likely to give out, and it must not keep the memory from being tuned.
     if (obj.mem_oc) {
-        const int ceiling = journal.ceilings().mem_mhz;
+        int ceiling = journal.ceilings().mem_mhz;
+        // Where the memory gave out in an earlier scan on this card and
+        // driver, the scan does not go again: it stops as far below it as
+        // confirming starts below a reset. A card whose bandwidth rises until
+        // its memory fails would otherwise end every run in a wrong result or
+        // a driver reset (hardware check 95).
+        if (const int edge = journal.mem_edge(); edge != INT_MAX) {
+            const int top = std::max(0, edge - kResetBackoffSteps * kMemStep);
+            log("mem: the memory gave out at " + offset_text(edge) + " in an earlier run on this card and driver; the scan stops at " +
+                offset_text(top));
+            ceiling = std::min(ceiling, top + 1);
+        }
         int reset_at = -1;   // the memory offset at which a reset event ended the scan
+        int gave_out_at = 0; // the offset whose candidate computed a wrong result
         const bool by_bandwidth = static_cast<bool>(io.bandwidth);
         bool left_at_stock = false;   // the measurement does not work: nothing was searched
         if (by_bandwidth) {
             bool unavailable = false;
-            const int peak = best_bandwidth_offset(0, bounds.mem_max_mhz, kMemStep, ceiling, [&](int v) {
+            // One step of either pass: the candidate, then the reading while
+            // it is still applied. `again`: in the second pass.
+            const auto sample = [&](int v, bool again) {
                 MemSample m;
+                // One reading. One that only did not settle is taken again,
+                // kBandwidthReadings times at most; any other failure ends
+                // the step at once.
                 std::optional<double> gbps;
+                bool failed = false;
                 // The +0 sample is journaled without a clock, like a soak at
                 // stock: if the machine froze in it, an open entry that said
                 // memory 0 would read as "never search memory again".
                 const std::optional<int> mem_j = v > 0 ? std::optional<int>(v) : std::nullopt;
                 m.stable = explore(v, reset_at, [&] {
-                    if (!is_stable(clock_candidate(std::nullopt, mem_j, 0, v, kClockProbeS, [&] { gbps = io.bandwidth(); })))
-                        return false;
+                    const auto verdict = clock_candidate(std::nullopt, mem_j, 0, v, kClockProbeS, [&] {
+                        for (int i = 0; i < kBandwidthReadings && !gbps && !failed; ++i) {
+                            gbps = io.bandwidth();
+                            failed = !gbps && !(io.bandwidth_unsettled && io.bandwidth_unsettled());
+                        }
+                    });
+                    if (verdict == Verdict::WrongResult) gave_out_at = v;
+                    if (!is_stable(verdict)) return false;
                     if (gbps) return true;
                     // The candidate passed, the measurement failed: the step is unusable.
-                    if (v > 0 && io.bandwidth_unsettled && io.bandwidth_unsettled()) {
+                    if (v > 0 && !failed) {
                         // The load computed and the readings disagreed: noise,
                         // not the card giving out. The scan ends with the best
                         // of what was measured; the core is still searched.
@@ -620,11 +677,21 @@ OptimizeResult optimize(const GpuControl& gpu, const Objectives& obj, Journal& j
                 if (m.stable) {
                     m.gbps = *gbps;
                     char buf[64];
-                    std::snprintf(buf, sizeof(buf), "mem +%d: bandwidth %.1f GB/s", v, m.gbps);
+                    std::snprintf(buf, sizeof(buf), "mem +%d: bandwidth %.1f GB/s%s", v, m.gbps, again ? ", read again" : "");
                     log(buf);
                 }
                 return m;
-            });
+            };
+            const MemScanEnd end = bandwidth_scan(0, bounds.mem_max_mhz, kMemStep, ceiling, [&](int v) { return sample(v, false); });
+            int peak = second_pass_top(end, 0, kMemStep, journal.mem_edge());
+            // The second pass, when there is something to read and the scan
+            // left the card in a state to read it: not after a reset event
+            // (nothing new is explored then, and confirming starts
+            // kResetBackoffSteps below `peak`), not on a stopped run.
+            if (!unavailable && stopped.empty() && !explored_enough && peak > 0) {
+                log("mem: the scan passed " + offset_text(end.top) + "; reading again from " + offset_text(peak) + " down");
+                peak = bandwidth_peak_below(peak, 0, kMemStep, [&](int v) { return sample(v, true); });
+            }
             // An unavailable measurement ended the scan at +0, and nothing
             // is searched without it. Memory that corrects its own errors
             // computes right far beyond the point where it stops getting
@@ -641,6 +708,12 @@ OptimizeResult optimize(const GpuControl& gpu, const Objectives& obj, Journal& j
                 reset_event(why);
             }
             left_at_stock = unavailable;
+            // The edge of this card's memory, for the runs after this one. A
+            // reset event counts as much as a wrong result: the scan ended
+            // there because the memory did.
+            if (reset_at > 0) gave_out_at = reset_at;
+            if (gave_out_at > 0 && !journal.note_mem_edge(gave_out_at))
+                log("warning: the memory's edge could not be written to the journal; the next run will find it again");
         }
         if (!by_bandwidth) {
             r.mem_max_stable = climb_to_edge(0, bounds.mem_max_mhz, kMemStep, kMemStride, ceiling, [&](int v) {
@@ -650,7 +723,7 @@ OptimizeResult optimize(const GpuControl& gpu, const Objectives& obj, Journal& j
         if (!stopped.empty()) return finish_fail(stopped);
         if (reset_at >= 0)
             log("mem: the driver reset at " + offset_text(reset_at) + "; using " + offset_text(r.mem_max_stable) +
-                (by_bandwidth ? ", the best of the values that passed" : ", the last value that passed"));
+                (by_bandwidth ? ", well below the values that passed" : ", the last value that passed"));
         r.mem_confirmed = confirm_edge(confirm_from(r.mem_max_stable, kMemStep, reset_at), 0, kMemStep, kConfirmTries, [&](int v) {
             return holds(clock_candidate(std::nullopt, v, 0, v, kConfirmProbeS));
         }, confirm_step_down);