java-topology/defects/redot/patch/redot-0005-astar-open-list-find-o1.patch

320 lines
12 KiB
Diff
Raw Permalink Blame History

This file contains ambiguous Unicode characters

This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.

# 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) — 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. 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 (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
@@ -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) {
@@ -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);
new_point = true;
} 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) { // 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);
}
}
@@ -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
@@ -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;