Skretzo / shortest-path

Pathfinding for Old School RuneScape
BSD 2-Clause "Simplified" License
14 stars 27 forks source link

Performance fixes #41

Closed jocopa3 closed 1 year ago

jocopa3 commented 1 year ago

Optimized pathfinding as much as I could without making any major refactors:

Results

Ran through a bunch of different paths short and long in-game to make sure the resulting path is still the same as OSRS's pathfinding.

Worst-Case test: from GE to top-left corner of the map in the middle of the ocean Short-Case test: from GE to a point just west 105 tiles away

Ran a few paths before-hand to warm-up the JVM before recording results. Screenshots are of the Intellij profiler of both CPU Samples and Memory Allocations

System used is an Intel i9-13900K with 64GB DDR5 RAM, OpenJDK 11.0.12.0 (a bit out of date). While not the average system by far, results should scale similarly on other machines.

Metric Before (1740a91) After (6137611) Difference
Worst-Case Time 2200.5582ms 1224.3621ms 976.1961ms
Short-Case Time 44.2161ms 28.5745ms 15.6416ms
Allocations 3,741,112,192 bytes 1,314,359,520 bytes 2,426,752,672 bytes

Before Optimizations

Worst-Case: 2200.5582ms Short-Case: 44.2161ms

CPU Samples (Worst-Case):

image

Memory Allocations (Worst-Case):

image

After Optimizations

Worst-Case: 1224.3621ms Short-Case: 28.5745ms

CPU Samples (Worst-Case):

image

Memory Allocations (Worst-Case):

image
Skretzo commented 1 year ago

I fixed two small bugs in a1f5ed56ff329663fa2cc62eebd15a63e6462463 and 1f0f115cedb05c528fa0fbb8d0343681d1321a3b, so there are a few conflicts you will need to resolve.

jocopa3 commented 1 year ago

Addressed comments and cleaned up a few minor things.

Skretzo commented 1 year ago

Could you please resolve the conflicts in ShortestPathPlugin.java, Pathfinder.java and PathfinderConfig.java so that this PR is up to date with your threading fixes that I merged

Skretzo commented 1 year ago

I can't seem to spot exactly where, but when testing this PR the Avoid wilderness option in the config no longer makes any difference for the pathfinding. When this option is enabled the path should not use the wilderness lever to go from Edgeville to Ardougne. pathfinding_avoid_wilderness_config_not_working

jocopa3 commented 1 year ago

the Avoid wilderness option in the config no longer makes any difference for the pathfinding

I've found and fixed this though there are some weird results like below; I'm not sure if this is expected or not but it seems like a quirk of the closest-point heuristic:

image

Performance wise in the worst-case, runtime speed is the same with/without due to the reduction in search space while allocations rose to 1,402,018,656 bytes from 1,314,359,520 bytes. Allocation impact can be eliminated with the WorldPoint packing optimization I've mentioned but I'm not going to do that in this PR.

image
Skretzo commented 1 year ago

I'm not sure if this is expected or not but it seems like a quirk of the closest-point heuristic

Believe it or not, but the path you got right there is more or less correct. You probably had Avoid wilderness enabled and Use fairy rings disabled. I have not added all agility shortcuts and teleports in the game yet, so the closest dungeon to the location you picked out in the ocean (cyan mark) was therefore Tarn's Lair (green circle), which requires a bit of a winding path through Morytania and the Haunted Mine when starting at the GE (blue circle) and with the Wilderness being forbidden (red cross).

image

If you want a better map than the world map in-game I would highly recommend https://mejrs.github.io/osrs. That map allows you to see all the dungeons up north of the main world, as well as displaying coordinates, labels, crowdsourced transport locations, objects, nomove tiles and npc information.