Poll timeout/interrupt/memory-limit inside dive loops; fix pulling should_stop - #27
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens several RCSPP algorithms against pathological “dive” behavior by adding periodic stop-condition polling inside deep inner loops, and fixes PullingDominanceAlgorithm to honor the same stop logic as the base dominance loop.
Changes:
- Add periodic polling (every 4096 steps) for timeout / external interrupt / memory-limit during DFS-style dive loops in tabu-search variants and greedy backtracking.
- Update
PullingDominanceAlgorithm::main_loop()to useshould_stop(i)instead of only checking the iteration budget. - Document the pathological long-run / OOM scenarios these guards address (ROADEF setB-01).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| cpp/rcspp/algorithm/tabu_search.hpp | Adds periodic timeout/interrupt/memory polling inside dive_to_sink() to avoid wedging in a single dive. |
| cpp/rcspp/algorithm/pulling_dominance_algorithm.hpp | Fixes override loop to use should_stop(i) so timeout/interrupt are respected. |
| cpp/rcspp/algorithm/improving_tabu_search.hpp | Adds the same periodic polling inside dive_to_sink() for the improving tabu search. |
| cpp/rcspp/algorithm/greedy.hpp | Adds periodic polling inside greedy backtracking/extension to avoid long uninterruptible runs. |
Suppressed comments (1)
cpp/rcspp/algorithm/greedy.hpp:109
- The 4096-step poll is currently only evaluated once per outer loop iteration, but the inner
while (extend_label(...))can perform an arbitrarily long greedy “dive” (many label allocations / extensions) without ever re-checking timeout/interrupt/memory. To actually make greedy dives interruptible, poll per extension attempt (e.g., inside the inner loop or by incrementingstepsfor each call to extend_label()).
bool extended = false;
while (extend_label(path_.back().first)) {
extended = true; // successfully extended
}
if (extended) {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+98
to
+104
| size_t steps = 0; | ||
| while (!path_.empty()) { | ||
| if ((++steps & 0xFFFU) == 0 && | ||
| (this->is_time_out() || this->is_interrupted() || | ||
| this->memory_limit_.is_exceeded())) { | ||
| return; | ||
| } |
Comment on lines
153
to
+158
| while (!this->path_.empty()) { | ||
| if ((++steps & 0xFFFU) == 0 && | ||
| (this->is_time_out() || this->is_interrupted() || | ||
| this->memory_limit_.is_exceeded())) { | ||
| return false; | ||
| } |
Comment on lines
+223
to
+229
| size_t steps = 0; | ||
| while (!this->path_.empty()) { | ||
| if ((++steps & 0xFFFU) == 0 && | ||
| (this->is_time_out() || this->is_interrupted() || | ||
| this->memory_limit_.is_exceeded())) { | ||
| return false; | ||
| } |
legraina
force-pushed
the
fix/dive-loop-stop-polling
branch
from
August 3, 2026 15:44
7bc2997 to
741c4de
Compare
dive_to_sink() in the tabu searches (and greedy extend) is a DFS with backtracking; on pathological instances a single dive can enumerate exponentially many partial paths while all per-iteration should_stop() checks sit between dives, so timeout_s and the external interrupt never fire and one solve can run unbounded. - greedy.hpp, tabu_search.hpp, improving_tabu_search.hpp: poll should_stop() every 4096 dive steps. - pulling_dominance_algorithm.hpp: main_loop now uses should_stop(i) (timeout + interrupt + budgets) like the base DominanceAlgorithm, instead of only the iteration budget. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Each dive extension allocates a label, so a pathological dive is an RSS runaway as well as a time sink, and only the dominance main loops consulted MemoryLimitHelper. The every-4096-steps dive poll now also checks is_exceeded(). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
legraina
force-pushed
the
fix/dive-loop-stop-polling
branch
from
August 3, 2026 20:57
741c4de to
b5665b5
Compare
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
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
Two fixes for pathological solves that can wedge a caller for minutes inside a single
solve()call.1. Poll should_stop() inside dive loops (7ca2273)
A dive is a DFS with backtracking: on pathological instances a single dive can enumerate exponentially many partial paths, while all per-iteration
should_stop()checks sit between dives — sotimeout_sand the external interrupt never fire. The dive loops inGreedyAlgorithm,TabuSearchAlgorithm, andImprovingTabuSearchnow pollshould_stop()every 4096 steps.Also fixes
PullingDominanceAlgorithm::main_loop, which historically only checked the iteration budget: it now usesshould_stop(i), which covers the budget plustimeout_sand the external interrupt, like the baseDominanceAlgorithm.2. Poll the memory limit inside dive loops (b5665b5)
Each dive extension allocates a label, so a pathological dive is an RSS runaway as well as a time sink — and only the dominance main loops consulted
MemoryLimitHelper. The same 4096-step poll now also checksmemory_limit_.is_exceeded(). The memory check stays out ofshould_stop()itself on purpose: reading process RSS is a real syscall, andshould_stop()runs every iteration of the hot dominance loops (which sample memory on their own throttledmemory_check_intervalcadence).🤖 Generated with Claude Code