freeciv-0001: assign_continent_flood() tile_list_search O(T^2), 53x
BFS flood fill in server/generator/mapgen_utils.c uses tile_list_search() (O(N) linked-list scan) as visited check per adjacent tile. On a continent of T tiles, each tile's 4-8 neighbors each trigger a linear scan of our growing worklist, making total complexity O(T * adj * T) = O(T^2). Fix: set tile_continent() at enqueue time instead of dequeue time. Our continent field itself becomes our visited set, replacing O(N) membership checks with O(1) integer comparisons. Worklist is now a pure FIFO queue. Measured: 53x overhead at T=25,600 tiles (160x160 map). Standard large Freeciv maps have continents of 5,000-20,000+ tiles. MOAD 0002 (Intertangle): Freeciv uses struct civ_game as global god object, expected for a single-threaded C game from 1996. Not a practical defect. MOAD 0003 (Leaked Context): CLEAN. Thread-local only in bundled tinycthread dependency. Tex AI uses proper mutexes. MOAD 0004 (Logged Secret): CLEAN. auth.c logs rejection messages with usernames only, never passwords or credentials. MOAD 0005 (Thundering Herd): CLEAN. AI settler cache uses hash lookup, single-threaded game loop has no concurrent cache races.
This commit is contained in:
parent
9177278be7
commit
c4154e0590
2 changed files with 267 additions and 0 deletions
45
defects/freeciv-0001/patch/freeciv-0001.patch
Normal file
45
defects/freeciv-0001/patch/freeciv-0001.patch
Normal file
|
|
@ -0,0 +1,45 @@
|
|||
--- a/server/generator/mapgen_utils.c
|
||||
+++ b/server/generator/mapgen_utils.c
|
||||
@@ -289,7 +289,8 @@
|
||||
/**********************************************************************//**
|
||||
Number this tile and nearby tiles with the specified continent number 'nr'.
|
||||
Due to the number of recursion for large maps a non-recursive algorithm is
|
||||
- utilised.
|
||||
+ utilised. Tiles are marked with their continent number when enqueued,
|
||||
+ eliminating O(N) membership checks on the worklist.
|
||||
|
||||
is_land tells us whether we are assigning continent numbers or ocean
|
||||
numbers.
|
||||
@@ -308,6 +309,9 @@
|
||||
&& T_UNKNOWN != pterrain
|
||||
&& XOR(is_land, terrain_type_terrain_class(pterrain) == TC_OCEAN));
|
||||
|
||||
+ /* Mark the initial tile immediately to avoid re-enqueue. */
|
||||
+ tile_set_continent(ptile, nr);
|
||||
+
|
||||
/* Create tile list and insert the initial tile. */
|
||||
tlist = tile_list_new();
|
||||
tile_list_append(tlist, ptile);
|
||||
@@ -325,15 +329,16 @@
|
||||
continue;
|
||||
}
|
||||
|
||||
- /* Add the tile to the list of tiles to check. */
|
||||
- if (!tile_list_search(tlist, ptile3)) {
|
||||
- tile_list_append(tlist, ptile3);
|
||||
- }
|
||||
+ /* Mark and enqueue. The continent field serves as our visited set,
|
||||
+ * replacing the O(N) tile_list_search() with an O(1) check above. */
|
||||
+ tile_set_continent(ptile3, nr);
|
||||
+ tile_list_append(tlist, ptile3);
|
||||
} adjc_iterate_end;
|
||||
|
||||
- /* Set the continent data and remove the tile from the list. */
|
||||
- tile_set_continent(ptile2, nr);
|
||||
+ /* Remove the tile from the worklist. Continent was already set
|
||||
+ * at enqueue time. */
|
||||
tile_list_remove(tlist, ptile2);
|
||||
+ /* Note: tile_list_get(tlist, 0) is O(1) for linked list head. */
|
||||
|
||||
/* Count the tile */
|
||||
if (nr < 0) {
|
||||
222
defects/freeciv-0001/test/test_continent_flood.c
Normal file
222
defects/freeciv-0001/test/test_continent_flood.c
Normal file
|
|
@ -0,0 +1,222 @@
|
|||
/*
|
||||
* Unit test for freeciv-0001: assign_continent_flood() BFS uses O(N)
|
||||
* tile_list_search() as visited check, making continent assignment O(T^2).
|
||||
*
|
||||
* Fix: mark tiles with continent number at enqueue time, replacing
|
||||
* the O(N) linked-list membership check with an O(1) integer check.
|
||||
*
|
||||
* We simulate a grid-based BFS flood fill on a square map and count
|
||||
* the number of "search operations" (comparisons) performed by each
|
||||
* approach.
|
||||
*/
|
||||
|
||||
#include <stdio.h>
|
||||
#include <stdlib.h>
|
||||
#include <string.h>
|
||||
#include <time.h>
|
||||
|
||||
/* Simulated tile: just needs a continent number and terrain type */
|
||||
struct tile {
|
||||
int continent;
|
||||
int is_land; /* 1 = land, 0 = ocean */
|
||||
};
|
||||
|
||||
/* Simple linked list node for worklist */
|
||||
struct node {
|
||||
int tile_idx;
|
||||
struct node *next;
|
||||
};
|
||||
|
||||
struct worklist {
|
||||
struct node *head;
|
||||
struct node *tail;
|
||||
int size;
|
||||
};
|
||||
|
||||
static struct worklist *wl_new(void) {
|
||||
struct worklist *wl = calloc(1, sizeof(*wl));
|
||||
return wl;
|
||||
}
|
||||
|
||||
static void wl_append(struct worklist *wl, int idx) {
|
||||
struct node *n = malloc(sizeof(*n));
|
||||
n->tile_idx = idx;
|
||||
n->next = NULL;
|
||||
if (wl->tail) {
|
||||
wl->tail->next = n;
|
||||
} else {
|
||||
wl->head = n;
|
||||
}
|
||||
wl->tail = n;
|
||||
wl->size++;
|
||||
}
|
||||
|
||||
static int wl_pop_front(struct worklist *wl) {
|
||||
struct node *n = wl->head;
|
||||
int idx = n->tile_idx;
|
||||
wl->head = n->next;
|
||||
if (!wl->head) wl->tail = NULL;
|
||||
free(n);
|
||||
wl->size--;
|
||||
return idx;
|
||||
}
|
||||
|
||||
/* O(N) linear search through linked list, counting comparisons */
|
||||
static int wl_search(struct worklist *wl, int idx, long *ops) {
|
||||
struct node *n = wl->head;
|
||||
while (n) {
|
||||
(*ops)++;
|
||||
if (n->tile_idx == idx) return 1;
|
||||
n = n->next;
|
||||
}
|
||||
return 0;
|
||||
}
|
||||
|
||||
static void wl_destroy(struct worklist *wl) {
|
||||
while (wl->head) {
|
||||
struct node *n = wl->head;
|
||||
wl->head = n->next;
|
||||
free(n);
|
||||
}
|
||||
free(wl);
|
||||
}
|
||||
|
||||
/* Get adjacent tile indices for a grid position (4-connected) */
|
||||
static int get_adjacent(int idx, int width, int height, int *adj) {
|
||||
int x = idx % width;
|
||||
int y = idx / width;
|
||||
int count = 0;
|
||||
|
||||
if (x > 0) adj[count++] = idx - 1;
|
||||
if (x < width - 1) adj[count++] = idx + 1;
|
||||
if (y > 0) adj[count++] = idx - width;
|
||||
if (y < height - 1) adj[count++] = idx + width;
|
||||
|
||||
return count;
|
||||
}
|
||||
|
||||
/*
|
||||
* DEFECTIVE: Uses tile_list_search (O(N) linked list scan) to check
|
||||
* if a tile is already in our worklist before adding it.
|
||||
*/
|
||||
static long flood_defective(struct tile *tiles, int width, int height,
|
||||
int start, int nr) {
|
||||
long ops = 0;
|
||||
struct worklist *wl = wl_new();
|
||||
int adj[4];
|
||||
|
||||
wl_append(wl, start);
|
||||
|
||||
while (wl->size > 0) {
|
||||
int current = wl_pop_front(wl);
|
||||
int nadj = get_adjacent(current, width, height, adj);
|
||||
|
||||
for (int i = 0; i < nadj; i++) {
|
||||
int neighbor = adj[i];
|
||||
if (tiles[neighbor].continent != 0 || !tiles[neighbor].is_land)
|
||||
continue;
|
||||
|
||||
/* DEFECT: O(N) search through linked list worklist */
|
||||
if (!wl_search(wl, neighbor, &ops)) {
|
||||
wl_append(wl, neighbor);
|
||||
}
|
||||
}
|
||||
|
||||
tiles[current].continent = nr;
|
||||
}
|
||||
|
||||
wl_destroy(wl);
|
||||
return ops;
|
||||
}
|
||||
|
||||
/*
|
||||
* PATCHED: Mark tiles with continent number at enqueue time.
|
||||
* The continent field itself serves as our visited set (O(1) check).
|
||||
*/
|
||||
static long flood_patched(struct tile *tiles, int width, int height,
|
||||
int start, int nr) {
|
||||
long ops = 0;
|
||||
struct worklist *wl = wl_new();
|
||||
int adj[4];
|
||||
|
||||
/* Mark immediately at enqueue */
|
||||
tiles[start].continent = nr;
|
||||
wl_append(wl, start);
|
||||
|
||||
while (wl->size > 0) {
|
||||
int current = wl_pop_front(wl);
|
||||
int nadj = get_adjacent(current, width, height, adj);
|
||||
|
||||
for (int i = 0; i < nadj; i++) {
|
||||
int neighbor = adj[i];
|
||||
ops++; /* O(1) integer comparison */
|
||||
if (tiles[neighbor].continent != 0 || !tiles[neighbor].is_land)
|
||||
continue;
|
||||
|
||||
/* PATCHED: O(1) mark-and-enqueue */
|
||||
tiles[neighbor].continent = nr;
|
||||
wl_append(wl, neighbor);
|
||||
}
|
||||
|
||||
/* No tile_list_remove needed, just popped from front */
|
||||
}
|
||||
|
||||
wl_destroy(wl);
|
||||
return ops;
|
||||
}
|
||||
|
||||
int main(void) {
|
||||
/* Test with increasing continent sizes */
|
||||
int sizes[] = {10, 20, 40, 80, 160};
|
||||
int nsizes = sizeof(sizes) / sizeof(sizes[0]);
|
||||
int all_pass = 1;
|
||||
|
||||
printf("freeciv-0001: assign_continent_flood() tile_list_search O(T^2)\n");
|
||||
printf("%-8s %-10s %-12s %-12s %-8s %s\n",
|
||||
"Size", "Tiles", "Defective", "Patched", "Ratio", "Status");
|
||||
printf("%-8s %-10s %-12s %-12s %-8s %s\n",
|
||||
"----", "-----", "---------", "-------", "-----", "------");
|
||||
|
||||
for (int s = 0; s < nsizes; s++) {
|
||||
int width = sizes[s];
|
||||
int height = sizes[s];
|
||||
int total = width * height;
|
||||
|
||||
/* Create an all-land map */
|
||||
struct tile *tiles_def = calloc(total, sizeof(struct tile));
|
||||
struct tile *tiles_pat = calloc(total, sizeof(struct tile));
|
||||
|
||||
for (int i = 0; i < total; i++) {
|
||||
tiles_def[i].is_land = 1;
|
||||
tiles_def[i].continent = 0;
|
||||
tiles_pat[i].is_land = 1;
|
||||
tiles_pat[i].continent = 0;
|
||||
}
|
||||
|
||||
long ops_def = flood_defective(tiles_def, width, height, 0, 1);
|
||||
long ops_pat = flood_patched(tiles_pat, width, height, 0, 1);
|
||||
|
||||
/* Verify all tiles were assigned */
|
||||
int assigned_def = 0, assigned_pat = 0;
|
||||
for (int i = 0; i < total; i++) {
|
||||
if (tiles_def[i].continent == 1) assigned_def++;
|
||||
if (tiles_pat[i].continent == 1) assigned_pat++;
|
||||
}
|
||||
|
||||
double ratio = (ops_pat > 0) ? (double)ops_def / ops_pat : 0;
|
||||
int pass = (assigned_def == total && assigned_pat == total
|
||||
&& ops_pat < ops_def);
|
||||
|
||||
printf("%-8d %-10d %-12ld %-12ld %-8.1fx %s\n",
|
||||
width, total, ops_def, ops_pat, ratio,
|
||||
pass ? "PASS" : "FAIL");
|
||||
|
||||
if (!pass) all_pass = 0;
|
||||
|
||||
free(tiles_def);
|
||||
free(tiles_pat);
|
||||
}
|
||||
|
||||
printf("\n%s\n", all_pass ? "ALL PASS" : "SOME FAILED");
|
||||
return all_pass ? 0 : 1;
|
||||
}
|
||||
Loading…
Add table
Add a link
Reference in a new issue