From 0bafd694ac200a8d628c7c7d1062776037c29342 Mon Sep 17 00:00:00 2001 From: "russell@unturf.com" Date: Tue, 7 Apr 2026 19:58:20 -0400 Subject: [PATCH] redot-0005: replace partial patch with full indexed heap implementation --- .../redot-0005-astar-open-list-find-o1.patch | 380 ++++++++++++++---- 1 file changed, 302 insertions(+), 78 deletions(-) diff --git a/defects/redot/patch/redot-0005-astar-open-list-find-o1.patch b/defects/redot/patch/redot-0005-astar-open-list-find-o1.patch index aa03a934c..c33a5d743 100644 --- a/defects/redot/patch/redot-0005-astar-open-list-find-o1.patch +++ b/defects/redot/patch/redot-0005-astar-open-list-find-o1.patch @@ -1,96 +1,320 @@ # UNDF: UNDF-2026-000001235 # CWE-407: Algorithmic Complexity — O(N²) → O(N log N) in AStar3D / AStar2D / AStarGrid2D # -# Defect: open_list.find(e) is O(N) — Vector linear scan — inside the A* decrease-key -# path, called once per neighbor relaxation. For a path of length P through a graph of -# N nodes, total cost: O(P × N) = O(N²) worst case. +# Defect: open_list.find(e) is O(N) — LocalVector linear scan — inside the A* decrease-key +# path, called once per neighbor relaxation when a cheaper route to an already-open node +# is found. For a path through N nodes with average degree D, worst case O(N × D × N) = O(N²). # -# Fix: add open_index field to Point struct. Maintained on push/pop so that -# decrease-key uses e->open_index — O(1) — instead of open_list.find(e). -# Affects AStar3D, AStar2D, AStarGrid2D — three classes, same fix. +# Fix: add open_index field to Point struct. Replace SortArray push_heap/pop_heap with +# custom indexed sift_up / sift_down that update open_index on every swap. Decrease-key +# uses e->open_index (O(1)) instead of open_list.find(e) (O(N)). +# Affects AStar3D::_solve(), AStar2D::_solve(), AStarGrid2D::_solve() — same fix in all three. # Total cost after: O(N log N) — standard A* complexity. # -# Complexity gate (unit/test-redot-0005-astar-open-list.cpp): -# N=1000-node grid: time ratio on 5× scale must be <17.5× - ---- a/core/math/a_star.h -+++ b/core/math/a_star.h -@@ -45,6 +45,7 @@ class AStar3D : public RefCounted { - struct Point { - Point() {} - -+ int32_t open_index = -1; // FIX redot-0005: heap position for O(1) decrease-key - int64_t id = 0; - Vector3 pos; - real_t weight_scale = 0; - +# Complexity gate (tests/core/math/test_astar.h, "[CWE-407][Complexity]" test): +# 50 corner-to-corner queries on a 40×50 (N=2000) grid must complete in <1.0 s. +diff --git a/core/math/a_star.cpp b/core/math/a_star.cpp +index 9d36a54c51..aa6aa85f1a 100644 --- a/core/math/a_star.cpp +++ b/core/math/a_star.cpp -@@ -338,12 +338,19 @@ bool AStar3D::_solve(Point *begin_point, Point *end_point, bool p_allow_partial_ - sorter.pop_heap(0, open_list.size(), open_list.ptr()); - open_list.remove_at(open_list.size() - 1); -+ p->open_index = -1; // no longer in heap - p->closed_pass = pass; - +@@ -35,6 +35,49 @@ + + #include "core/math/geometry_3d.h" + ++// Indexed min-heap helpers for A* open list. ++// Maintain Point::open_index on every swap so decrease-key is O(1) instead of O(N). ++template ++static void _astar_sift_up(LocalVector &heap, int pos) { ++ Cmp cmp; ++ while (pos > 0) { ++ int parent = (pos - 1) / 2; ++ if (!cmp(heap[parent], heap[pos])) { ++ break; // parent is not worse than pos — heap invariant holds ++ } ++ PT *tmp = heap[parent]; ++ heap[parent] = heap[pos]; ++ heap[pos] = tmp; ++ heap[parent]->open_index = parent; ++ heap[pos]->open_index = pos; ++ pos = parent; ++ } ++} ++ ++template ++static void _astar_sift_down(LocalVector &heap, int pos) { ++ Cmp cmp; ++ int size = (int)heap.size(); ++ while (true) { ++ int best = pos, left = 2 * pos + 1, right = 2 * pos + 2; ++ if (left < size && cmp(heap[best], heap[left])) { ++ best = left; ++ } ++ if (right < size && cmp(heap[best], heap[right])) { ++ best = right; ++ } ++ if (best == pos) { ++ break; ++ } ++ PT *tmp = heap[best]; ++ heap[best] = heap[pos]; ++ heap[pos] = tmp; ++ heap[best]->open_index = best; ++ heap[pos]->open_index = pos; ++ pos = best; ++ } ++} ++ + int64_t AStar3D::get_available_point_id() const { + if (points.has(last_free_id)) { + int64_t cur_new_id = last_free_id + 1; +@@ -319,12 +362,12 @@ bool AStar3D::_solve(Point *begin_point, Point *end_point, bool p_allow_partial_ + bool found_route = false; + + LocalVector open_list; +- SortArray sorter; + + begin_point->g_score = 0; + begin_point->f_score = _estimate_cost(begin_point->id, end_point->id); + begin_point->abs_g_score = 0; + begin_point->abs_f_score = _estimate_cost(begin_point->id, end_point->id); ++ begin_point->open_index = 0; + open_list.push_back(begin_point); + + while (!open_list.is_empty()) { +@@ -340,8 +383,16 @@ bool AStar3D::_solve(Point *begin_point, Point *end_point, bool p_allow_partial_ + break; + } + +- sorter.pop_heap(0, open_list.size(), open_list.ptr()); // Remove the current point from the open list. +- open_list.remove_at(open_list.size() - 1); ++ // Pop min: swap root with last element, remove last, sift root down. ++ int _last = (int)open_list.size() - 1; ++ Point *_tmp = open_list[0]; ++ open_list[0] = open_list[_last]; ++ open_list[_last] = _tmp; ++ open_list[0]->open_index = 0; ++ open_list.remove_at(_last); ++ if (!open_list.is_empty()) { ++ _astar_sift_down(open_list, 0); ++ } + p->closed_pass = pass; // Mark the point as closed. + for (const KeyValue &kv : p->neighbors) { - Point *e = kv.value; - - if (!e->enabled || e->closed_pass == pass) { - continue; - } - - real_t tentative_g_score = p->g_score + _compute_cost(p->id, e->id) * e->weight_scale; - - bool new_point = false; - - if (e->open_pass != pass) { +@@ -364,6 +415,7 @@ bool AStar3D::_solve(Point *begin_point, Point *end_point, bool p_allow_partial_ + + if (e->open_pass != pass) { // The point wasn't inside the open list. e->open_pass = pass; ++ e->open_index = (int)open_list.size(); open_list.push_back(e); -+ e->open_index = open_list.size() - 1; new_point = true; - } else if (tentative_g_score >= e->g_score) { - continue; - } - - e->prev_point = p; - e->g_score = tentative_g_score; - e->f_score = e->g_score + _estimate_cost(e->id, end_point->id); + } else if (tentative_g_score >= e->g_score) { // The new path is worse than the previous. +@@ -376,11 +428,11 @@ bool AStar3D::_solve(Point *begin_point, Point *end_point, bool p_allow_partial_ e->abs_g_score = tentative_g_score; e->abs_f_score = e->f_score - e->g_score; - - if (new_point) { - sorter.push_heap(0, open_list.size() - 1, 0, e, open_list.ptr()); - } else { -- sorter.push_heap(0, open_list.find(e), 0, e, open_list.ptr()); // O(N) find -+ sorter.push_heap(0, e->open_index, 0, e, open_list.ptr()); // O(1) - } + +- if (new_point) { // The position of the new points is already known. +- sorter.push_heap(0, open_list.size() - 1, 0, e, open_list.ptr()); +- } else { +- sorter.push_heap(0, open_list.find(e), 0, e, open_list.ptr()); +- } ++ // Sift up from the element's known index — O(log N) whether new or updated. ++ // For new points, open_index was set above. For existing points, open_index ++ // was maintained by prior heap operations — no O(N) scan needed. ++ (void)new_point; ++ _astar_sift_up(open_list, e->open_index); } } - -@@ -875,7 +875,7 @@ bool AStar2D::_solve(AStar3D::Point *begin_point, AStar3D::Point *end_point, boo - if (new_point) { - sorter.push_heap(0, open_list.size() - 1, 0, e, open_list.ptr()); - } else { -- sorter.push_heap(0, open_list.find(e), 0, e, open_list.ptr()); // O(N) find -+ sorter.push_heap(0, e->open_index, 0, e, open_list.ptr()); // O(1) - } - ---- a/core/math/a_star_grid_2d.h -+++ b/core/math/a_star_grid_2d.h -@@ -76,6 +76,7 @@ class AStarGrid2D : public RefCounted { - struct Point { - Vector2i id; - -+ int32_t open_index = -1; // FIX redot-0005: heap position for O(1) decrease-key - Vector2 pos; - real_t weight_scale = 1.0; - + +@@ -853,12 +905,12 @@ bool AStar2D::_solve(AStar3D::Point *begin_point, AStar3D::Point *end_point, boo + bool found_route = false; + + LocalVector open_list; +- SortArray sorter; + + begin_point->g_score = 0; + begin_point->f_score = _estimate_cost(begin_point->id, end_point->id); + begin_point->abs_g_score = 0; + begin_point->abs_f_score = _estimate_cost(begin_point->id, end_point->id); ++ begin_point->open_index = 0; + open_list.push_back(begin_point); + + while (!open_list.is_empty()) { +@@ -874,8 +926,16 @@ bool AStar2D::_solve(AStar3D::Point *begin_point, AStar3D::Point *end_point, boo + break; + } + +- sorter.pop_heap(0, open_list.size(), open_list.ptr()); // Remove the current point from the open list. +- open_list.remove_at(open_list.size() - 1); ++ // Pop min: swap root with last element, remove last, sift root down. ++ int _last = (int)open_list.size() - 1; ++ AStar3D::Point *_tmp = open_list[0]; ++ open_list[0] = open_list[_last]; ++ open_list[_last] = _tmp; ++ open_list[0]->open_index = 0; ++ open_list.remove_at(_last); ++ if (!open_list.is_empty()) { ++ _astar_sift_down(open_list, 0); ++ } + p->closed_pass = astar.pass; // Mark the point as closed. + + for (KeyValue &kv : p->neighbors) { +@@ -898,6 +958,7 @@ bool AStar2D::_solve(AStar3D::Point *begin_point, AStar3D::Point *end_point, boo + + if (e->open_pass != astar.pass) { // The point wasn't inside the open list. + e->open_pass = astar.pass; ++ e->open_index = (int)open_list.size(); + open_list.push_back(e); + new_point = true; + } else if (tentative_g_score >= e->g_score) { // The new path is worse than the previous. +@@ -910,11 +971,9 @@ bool AStar2D::_solve(AStar3D::Point *begin_point, AStar3D::Point *end_point, boo + e->abs_g_score = tentative_g_score; + e->abs_f_score = e->f_score - e->g_score; + +- if (new_point) { // The position of the new points is already known. +- sorter.push_heap(0, open_list.size() - 1, 0, e, open_list.ptr()); +- } else { +- sorter.push_heap(0, open_list.find(e), 0, e, open_list.ptr()); +- } ++ // Sift up from the element's known index — O(log N) whether new or updated. ++ (void)new_point; ++ _astar_sift_up(open_list, e->open_index); + } + } + +diff --git a/core/math/a_star.h b/core/math/a_star.h +index 1f522cfa21..d93482b0ea 100644 +--- a/core/math/a_star.h ++++ b/core/math/a_star.h +@@ -61,6 +61,7 @@ class AStar3D : public RefCounted { + real_t f_score = 0; + uint64_t open_pass = 0; + uint64_t closed_pass = 0; ++ int32_t open_index = -1; // Index in open_list heap; maintained for O(1) decrease-key. + + // Used for getting closest_point_of_last_pathing_call. + real_t abs_g_score = 0; +diff --git a/core/math/a_star_grid_2d.cpp b/core/math/a_star_grid_2d.cpp +index 9044e5d605..17b8857ac4 100644 --- a/core/math/a_star_grid_2d.cpp +++ b/core/math/a_star_grid_2d.cpp -@@ -567,7 +567,7 @@ bool AStarGrid2D::_solve(Point *p_begin_point, Point *p_end_point) { - if (new_point) { - sorter.push_heap(0, open_list.size() - 1, 0, e, open_list.ptr()); - } else { -- sorter.push_heap(0, open_list.find(e), 0, e, open_list.ptr()); // O(N) find -+ sorter.push_heap(0, e->open_index, 0, e, open_list.ptr()); // O(1) - } +@@ -35,6 +35,49 @@ + + #include "core/variant/typed_array.h" + ++// Indexed min-heap helpers for A* open list. ++// Maintain Point::open_index on every swap so decrease-key is O(1) instead of O(N). ++template ++static void _astar_grid_sift_up(LocalVector &heap, int pos) { ++ Cmp cmp; ++ while (pos > 0) { ++ int parent = (pos - 1) / 2; ++ if (!cmp(heap[parent], heap[pos])) { ++ break; // parent is not worse than pos — heap invariant holds ++ } ++ PT *tmp = heap[parent]; ++ heap[parent] = heap[pos]; ++ heap[pos] = tmp; ++ heap[parent]->open_index = parent; ++ heap[pos]->open_index = pos; ++ pos = parent; ++ } ++} ++ ++template ++static void _astar_grid_sift_down(LocalVector &heap, int pos) { ++ Cmp cmp; ++ int size = (int)heap.size(); ++ while (true) { ++ int best = pos, left = 2 * pos + 1, right = 2 * pos + 2; ++ if (left < size && cmp(heap[best], heap[left])) { ++ best = left; ++ } ++ if (right < size && cmp(heap[best], heap[right])) { ++ best = right; ++ } ++ if (best == pos) { ++ break; ++ } ++ PT *tmp = heap[best]; ++ heap[best] = heap[pos]; ++ heap[pos] = tmp; ++ heap[best]->open_index = best; ++ heap[pos]->open_index = pos; ++ pos = best; ++ } ++} ++ + static real_t heuristic_euclidean(const Vector2i &p_from, const Vector2i &p_to) { + real_t dx = (real_t)Math::abs(p_to.x - p_from.x); + real_t dy = (real_t)Math::abs(p_to.y - p_from.y); +@@ -513,13 +556,13 @@ bool AStarGrid2D::_solve(Point *p_begin_point, Point *p_end_point, bool p_allow_ + int64_t traversal_count = 0; + + LocalVector open_list; +- SortArray sorter; + LocalVector nbors; + + p_begin_point->g_score = 0; + p_begin_point->f_score = _estimate_cost(p_begin_point->id, p_end_point->id); + p_begin_point->abs_g_score = 0; + p_begin_point->abs_f_score = _estimate_cost(p_begin_point->id, p_end_point->id); ++ p_begin_point->open_index = 0; + open_list.push_back(p_begin_point); + end = p_end_point; + +@@ -544,8 +587,16 @@ bool AStarGrid2D::_solve(Point *p_begin_point, Point *p_end_point, bool p_allow_ + // Increment traversals for each node we process. + traversal_count++; + +- sorter.pop_heap(0, open_list.size(), open_list.ptr()); // Remove the current point from the open list. +- open_list.remove_at(open_list.size() - 1); ++ // Pop min: swap root with last element, remove last, sift root down. ++ int _last = (int)open_list.size() - 1; ++ Point *_tmp = open_list[0]; ++ open_list[0] = open_list[_last]; ++ open_list[_last] = _tmp; ++ open_list[0]->open_index = 0; ++ open_list.remove_at(_last); ++ if (!open_list.is_empty()) { ++ _astar_grid_sift_down(open_list, 0); ++ } + p->closed_pass = pass; // Mark the point as closed. + + nbors.clear(); +@@ -572,6 +623,7 @@ bool AStarGrid2D::_solve(Point *p_begin_point, Point *p_end_point, bool p_allow_ + + if (e->open_pass != pass) { // The point wasn't inside the open list. + e->open_pass = pass; ++ e->open_index = (int)open_list.size(); + open_list.push_back(e); + new_point = true; + } else if (tentative_g_score >= e->g_score) { // The new path is worse than the previous. +@@ -585,11 +637,11 @@ bool AStarGrid2D::_solve(Point *p_begin_point, Point *p_end_point, bool p_allow_ + e->abs_g_score = tentative_g_score; + e->abs_f_score = e->f_score - e->g_score; + +- if (new_point) { // The position of the new points is already known. +- sorter.push_heap(0, open_list.size() - 1, 0, e, open_list.ptr()); +- } else { +- sorter.push_heap(0, open_list.find(e), 0, e, open_list.ptr()); +- } ++ // Sift up from the element's known index — O(log N) whether new or updated. ++ // For new points, open_index was set above. For existing points, open_index ++ // was maintained by prior heap operations — no O(N) scan needed. ++ (void)new_point; ++ _astar_grid_sift_up(open_list, e->open_index); + } + } + +diff --git a/core/math/a_star_grid_2d.h b/core/math/a_star_grid_2d.h +index 3363042a0c..52d77dc76a 100644 +--- a/core/math/a_star_grid_2d.h ++++ b/core/math/a_star_grid_2d.h +@@ -88,6 +88,7 @@ private: + real_t f_score = 0; + uint64_t open_pass = 0; + uint64_t closed_pass = 0; ++ int32_t open_index = -1; // Index in open_list heap; maintained for O(1) decrease-key. + + // Used for getting last_closest_point. + real_t abs_g_score = 0;