dist(), att_reacting_units() and s_commod() are only interested in
cost, not the actual path. BestLandPath() and BestDistPath() compute
both cost and path. Use path_find() directly instead.
Destinations are no longer treated as unreachable when the best path
is longer than 1023 characters.
bestownedpath() is a rather simple-minded breadth-first search. It's
slower than the new path finder, and maintaining it in addition to the
new path finder makes no sense.
This way, we compute all distribution paths from the same center in
one go, and thus fully exploit the fast multiple paths from same
source capability of Dijkstra's algorithm.
Sorting by dist center increases the average length of runs from 4.5
to 73 for my continental test case, and from 3 to 10 for my island
test case.
Compared to the commit before the previous one, distribution path
assembly runs more than 40 times faster for my continental test case,
and more than 5 times faster for my island test case.
The new path finder now runs my continental test case more than 30
times faster than the old A*, and the island test case more than 6
times, in a fraction of the memory. This makes the continental
updates run 3.5 times faster, and the island updates 6% faster.
Distribution path assembly no longer dominates the continental
update's run time: it takes less than 10% instead of more than 70%.
In a sense, this is the path cache done right.
Dijkstra's algorithm can find multiple paths from the same source.
This is much faster than starting from scratch for every path.
Make distribution path assembly work that way. This speeds up runs of
distributions to the same center. The next commit will reorder path
searches to maximize the length of these runs. It also has benchmark
results.
Allocates four bytes per sector, actually uses only the first 4*n
bytes, where n is the number of distributing sectors.
Because the cost to enter a sector is independent of the direction of
entry, we visit sectors at most once. Exploit that.
Beware: this is not the case for A*. Pitfall for any future
generalization to A*.
Speeds up distribution path assembly by 35-40% in my tests.
This gets rid of the memory leak mentioned in the previous commit.
To get rid of the buffer overruns for long paths mentioned in the
previous commit, make BestLandPath() fail when path length exceeds
1023 characters.
assemble_dist_paths() and move_ground() pass buffers with a different
size. Eliminate assemble_dist_paths()'s buffer. Update now works
regardless of distribution distance (the distribute command still
limits to 1023, to be fixed in a later commit). Enlarge
move_ground()'s buffers. Doubles the length of paths accepted by
explore, move, and transport.
I use two test cases to benchmark the path finders: "continental" (Hvy
Metal 2 updates) and "island" (Hvy Plastic 2 updates).
The new path finder runs my tests around 3-4 times faster than the old
A* without its caches. That's enough to meet its cached performance
for "island", but it's only half as fast for "continental". Not for
long; big speedups are coming.
We've been using Phil Lapsley's A* library to find land paths since
Chainsaw 3. It's reasonably general, and uses relatively complex data
structures to conserve memory. Unfortunately, it occasionally leaks a
bit of memory (see commit 86a187c0), and is unsafe for long paths (see
commit e30dc417).
To speed it up, v4.2.2 added two caches: the neighbor cache and the
path cache.
The neighbor cache attempts to speed up lookup of adjacent sectors.
It allocates 6 pointers per sector for that. In my tests, this is
more, sometimes much more memory than the A* library uses. See commit
7edcd3ea on branch old-astar for its (modest) performance impact.
The path cache attempts to speed up the update's computation of
distribution path costs. There, A* runs many times. Each run finds
many shortest paths, of which only the one asked for is returned. The
path cache saves all of them, so that when one of them is needed
later, we can get it from the path cache instead of running A* again.
The cache is quite effective, but a bit of a memory hog (see commit
a02d3e9f on branch old-astar).
I'm pretty sure I could speed up the path cache even more by reducing
its excessive memory consumption --- why store paths when we're only
interested in cost? But that's a bad idea, because the path cache
itself is a bad idea.
Finding many shortest paths from the same source has a well-known
efficient and simple solution: Dijkstra's algorithm[*].
A* is an extension of Dijkstra's algorithm. It computes a *single*
path faster than Dijkstra's. But it can't compute *many* shortest
paths from the same source as efficiently as Dijkstra's.
I could try to modify Phil's code to make it compute many shortest
paths from the same source efficiently: turn A* into its special case
Dijkstra's algorithm (at least for distribution path assembly), then
generalize it to the many paths problem. Of course, I'd also have to
track down its memory allocation bugs, and make it safe for long
paths.
Instead, I'm replacing it. This commit is the first step: a rather
unsophisticated implementation of Dijkstra's algorithm specialized to
hex maps. It works with simple data structures: an array for the hex
map (16 bytes per sector), and a binary heap for the priority queue
(16 bytes per sector, most of it never touched). This is more memory
than Phil's A* uses, but much less than Phil's A* with v4.2.2's
caches.
[*] To fully exploit Dijkstra's "many paths" capability, we need to
compute distribution paths in distribution center order.
Why upgrade? I'm not a lawyer, but here's my take on the differences
to version 2:
* Software patents: better protection against abuse of patents to
prevent users from exercising the rights under the GPL. I doubt
we'll get hit with a patent suit, but it's a good move just on
general principles.
* License compatibility: compatible with more free licenses, i.e. can
"steal" more free software for use in Empire. I don't expect to steal
much, but it's nice to have the option.
* Definition of "source code": modernization of some details for today's
networked world, to make it easier to distribute the software. Not
really relevant to us now, as we normally distribute full source code.
* Tivoization: this is about putting GPL-licensed software in hardware,
then make the hardware refuse to run modified software. "Neat" trick
to effectively deny its users their rights under the GPL. Abuse was
"pioneered" by TiVo (popular digital video recorders). GPLv3 forbids
it. Unlikely to become a problem for us.
* Internationalization: more careful wording, to harden the license
outside the US. The lawyers tell us it better be done that way.
* License violations: friendlier way to deal with license violations.
This has come out of past experience enforcing the GPL.
* Additional permissions: Probably not relevant to us.
Also include myself in the list of principal authors.
BestLandPath(), BestDistPath() and best_path() are unsafe by design:
they take a path[] argument without a size, and blindly assume there's
enough space. When that's wrong, bp_path() overruns the caller's
buffer.
move_ground() and assemble_dist_paths() provide space for 512
characters. best(), dist(), path(), att_reacting_units(), s_commod()
and do_unit_move() provide space for 1024 characters.
A malicious player can arrange paths longer than that, but it takes a
lot of land.
BestAirPath() and BestShipPath() also take a path[] argument without a
size, but they're actually safe: bestownedpath() writes at most 100
(MAXROUTE) characters, perform_mission_bomb() provides space for 512,
sorde(), getpath(), do_unit_move() and nav_ship() for 1024.
You can't distribute to a foreign sector. This case is relatively
rare. However, unsuccessful path search is relatively expensive, and
the extra check doesn't really slow down the common case.
The recovery avoided crashing here, but left the path costs undefined.
If they happend to be non-negative, dodistribute() still crashed. Set
the costs to -1 to avoid that.
While there, oops on invalid distribution center.
Import and export paths enter the same sectors, except for the last
one. Compute export cost from import cost instead of reverting the
import path. Do it in dodistribute(), so that we need to store only
import costs.
Since 4.2.2, assemble_dist_paths() stores a dummy path instead of the
real path to the dist center. That's possible because distribution
doesn't actually use the path, only whether it exists.
The code to store and free the real path is still around, under #ifdef
SAVE_FINISH_PATHS. Remove it.
World-sized bitmaps were allocated with size WORLD_SZ() / 8, which
expands to (WORLD_X * WORLD_Y / 2) / 8. The divisions truncate unless
WORLD_X * WORLD_Y is a multiple of 16. The bitmaps were one byte too
small then. Bitmap overruns happen when:
* A lookout looks at one of the last sectors of the sector file.
Besides commands look and llook, this affects navigate and march
sub-command 'l'.
* Command spy spies into one of the last sectors of the sector file.
* A map or nmap (but not a bmap) shows one of the last sectors of the
sector file, or a sector that can see one of the last sectors
(visual range is two sectors at 100% efficiency). Besides commands
lmap, map, nmap, pmap, smap, this affects move and transport
sub-command 'm'.
Diagnosed with valgrind.
Already broken in BSD Empire 1.1 (bitmaps were on the stack then).
We know player != other. Because we can have only one player in state
PS_PLAYING per country, and we know other->state == PS_PLAYING, it
follows that player->cnum != other->cnum. Thus, no functional change.
Adds another call to getnatp() hidden in relations_with(), though.
Keeping that optimized isn't worth it.
Replacing getrel(NP, THEM), where NP is known to be getnatp(US), by
relations_with(US, THEM) makes a difference only when US equals THEM.
Replace in places where it's obvious that they're not equal.
Adds a few calls to getnatp() hidden in relations_with(). Keeping
that optimized isn't worth it.
Replace patterns like "US == THEM || getrel(NP, THEM)...", where NP is
known to be getnatp(US), by "relations_with(US, THEM)...". No
functional change.
Adds a few calls to getnatp() hidden in relations_with(), though.
Keeping that optimized isn't worth it.
No functional change, because the value of rel only matters when cn !=
victim, and then it's the same as before.
The new value of rel permits simplifying cn != victim && rel <=
NEUTRAL to just rel <= NEUTRAL
This removes a special case for POGO (#0). Before, unoccupied sectors
were treated as "own or allied" for POGO, but not for other deities.
Impact on callers:
* BestAirPath() is not affected, because the change is only reachable
with a non-null bigmap argument.
* sorde() and nav_ship() pass a non-zero ship owner. sorde() ensures
that itself, and prod_ship() does it for nav_ship().
* unit_path() passes the player number when called with a ship
argument, i.e. in the navigate command. Player number is zero for
POGO. Since deities can't navigate foreign ships, this can happen
only when POGO navigates dead ships. Yes, that's possible, needs
fixing.
* getpath() passes the player number (zero for POGO) when called with
argument P_SAILING, i.e. by the sail command.
Thus, the change makes navigate's and sail's path finding work for
POGO exactly like it does for other deities. That's fine.
No functional change, even though this changes rel[intruder] from
NEUTRAL to ALLIED. Uses of rel[]:
* getilists() and ac_encounter() compare rel[cn] to HOSTILE. No
change, because NEUTRAL and ALLIED are both greater than HOSTILE.
* ac_encounter() compares rel[cn] to ALLIED, but only when cn !=
plane_owner. Because it passes plane_owner as argument for
getilists() parameter intruder, rel[cn] can't refer to the changed
element of rel[] here.
The new value of rel[plane_owner] permits simplifying cn ==
plane_owner || rel[cn] == ALLIED to just rel[cn] == ALLIED.
No functional change, because the value of rel only matters when
sectp->sct_own != sp->shp_own, and then it's the same as before.
The new value of rel permits simplifying sectp->sct_own == sp->shp_own
|| rel >= FRIENDLY to just rel >= FRIENDLY.
No functional change, because the change affects only
notified[victim], which isn't used in the loop around
notify_coastguard(), and gets overwritten before the interdiction fire
loop.
No functional change, because the value of rel only matters when
sect.sct_own != actor, and then it's the same as before.
The new value of rel permits simplifying sect.sct_own != actor && rel
!= ALLIED to just rel != ALLIED.
Replacing getrel(getnatp(US), THEM) by relations_with(US, THEM) makes
a difference only when US equals THEM. Replace in places where it's
obvious that they're not equal.
Note: getsect() sets player->owner to "player is god or owns this
sector". Thus, after getsect(..., §), sect.sct_own ==
player->cnum implies player->owner. Conversely, !player->owner
implies sect.sct_own != player->cnum. Similarly for getship(),
getplane() and nxtitem().
Switching from getrel() to relations_with() can change the value from
NEUTRAL to ALLIED. The change doesn't matter when the value's only
compared to HOSTILE, as both old and new value are greater than
HOSTILE. Likewise for >= NEUTRAL.
Replacing getrel(getnatp(US), THEM) by relations_with(US, THEM) makes
a difference only when US equals THEM.
Replace patterns like "us == them || getrel(getnatp(us), them)..." by
"relations_with(us, them)...".
