redot-0005: replace partial patch with full indexed heap implementation

This commit is contained in:
russell@unturf.com 2026-04-07 19:58:20 -04:00
parent 716ef7cb2d
commit 0bafd694ac

View file

@ -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 <typename PT, typename Cmp>
+static void _astar_sift_up(LocalVector<PT *> &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 <typename PT, typename Cmp>
+static void _astar_sift_down(LocalVector<PT *> &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<Point *> open_list;
- SortArray<Point *, SortPoints> 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<Point, SortPoints>(open_list, 0);
+ }
p->closed_pass = pass; // Mark the point as closed.
for (const KeyValue<int64_t, Point *> &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<Point, SortPoints>(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<AStar3D::Point *> open_list;
- SortArray<AStar3D::Point *, AStar3D::SortPoints> 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<AStar3D::Point, AStar3D::SortPoints>(open_list, 0);
+ }
p->closed_pass = astar.pass; // Mark the point as closed.
for (KeyValue<int64_t, AStar3D::Point *> &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<AStar3D::Point, AStar3D::SortPoints>(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 <typename PT, typename Cmp>
+static void _astar_grid_sift_up(LocalVector<PT *> &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 <typename PT, typename Cmp>
+static void _astar_grid_sift_down(LocalVector<PT *> &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<Point *> open_list;
- SortArray<Point *, SortPoints> sorter;
LocalVector<Point *> 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<Point, SortPoints>(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<Point, SortPoints>(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;