AStar Algorithm

Performance Tuning
#1

I designed my own aStar algorithm but i am afraid it is running a bit slow. Can anyone please look at it and let me know how it can be improved.


public class PathFinder {
	private int endX, endY;
	private int startX, startY;
	
	private int[][] map;
	
	private ArrayList<Node> open = new ArrayList<Node>();
	public static ArrayList<Node> closed = new ArrayList<Node>();
	
	public PathFinder(int[][] map){
		this.map = map;
	}
	
	public ArrayList<Node> findPath(Node start, Node end){
		open.clear();
		closed.clear();
		
		this.endX = end.x;
		this.endY = end.y;
		
		this.startX = start.x;
		this.startY = start.y;
		
		open.add(start);
		
		while (open.size() > 0){
			if (open.get(0).x == endX && open.get(0).y == endY){
				closed.add(open.get(0));
				open.remove(0);
				break;
			}else{
				addAdj(open.get(0));
			}
		}
		
		return reconstructPath();
	}
	
	long startTime;
	public ArrayList<Node> findPath(int startX, int startY, int endX, int endY){
		
		open.clear();
		closed.clear();
		
		this.endX = endX;
		this.endY = endY;
		
		this.startX = startX;
		this.startY = startY;
		
		open.add(new Node(startX, startY, startX, startY, 0));
		
		while (open.size() > 0){
			if (open.get(0).x == endX && open.get(0).y == endY){
				closed.add(open.get(0));
				open.remove(0);
				break;
			}else{
				addAdj(open.get(0));
				
			}
		}
		
		return reconstructPath();
	}
	
	private void addAdj(Node parent){
		closed.add(open.get(0));
		open.remove(0);
		
		if (map[parent.y+1][parent.x] != 1 && notClosed(parent.x,parent.y+1,Math.abs(endX-parent.x)+Math.abs(endY-(parent.y+1))+parent.fCost,parent.x,parent.y)){
			if (open.size()==0){
//				long startTime = System.nanoTime();
				open.add(new Node(parent.x,parent.y+1, parent.x, parent.y, Math.abs(endX-parent.x)+Math.abs(endY-(parent.y+1))+parent.fCost));
//				System.out.println("add time" + (System.nanoTime()-startTime));
			}else if (notQueued(parent.x,parent.y+1)){
				startTime = System.nanoTime();
				for (int i=0; i<open.size(); i++){
					if (Math.abs(endX-parent.x)+Math.abs(endY-(parent.y+1))+parent.fCost<open.get(i).fCost){
						open.add(i, new Node(parent.x,parent.y+1, parent.x, parent.y, Math.abs(endX-parent.x)+Math.abs(endY-(parent.y+1))+parent.fCost));
						break;
					}else if (i==open.size()-1){
						open.add(new Node(parent.x,parent.y+1, parent.x, parent.y, Math.abs(endX-parent.x)+Math.abs(endY-(parent.y+1))+parent.fCost));
						break;
					}	
				}
			}
		}if (map[parent.y-1][parent.x] != 1 && notClosed(parent.x,parent.y-1,Math.abs(endX-parent.x)+Math.abs(endY-(parent.y-1))+parent.fCost,parent.x,parent.y)){
			if (open.size()==0){
				open.add(new Node(parent.x,parent.y-1, parent.x, parent.y, Math.abs(endX-parent.x)+Math.abs(endY-(parent.y-1))+parent.fCost));
			}else if (notQueued(parent.x,parent.y-1)){
				for (int i=0; i<open.size(); i++){
					if (Math.abs(endX-parent.x)+Math.abs(endY-(parent.y-1))+parent.fCost<open.get(i).fCost){
						open.add(i, new Node(parent.x,parent.y-1, parent.x, parent.y, Math.abs(endX-parent.x)+Math.abs(endY-(parent.y-1))+parent.fCost));
						break;
					}else if (i==open.size()-1){
						open.add(new Node(parent.x,parent.y-1, parent.x, parent.y, Math.abs(endX-parent.x)+Math.abs(endY-(parent.y-1))+parent.fCost));
						break;
					}
				}
			}
		}if (map[parent.y][parent.x+1] != 1 && notClosed(parent.x+1,parent.y,Math.abs(endX-(parent.x+1))+Math.abs(endY-parent.y)+parent.fCost,parent.x,parent.y) ){
			if (open.size()==0){
				open.add(new Node(parent.x+1,parent.y, parent.x, parent.y, Math.abs(endX-(parent.x+1))+Math.abs(endY-parent.y)+parent.fCost));
			}else if (notQueued(parent.x+1,parent.y)){
				for (int i=0; i<open.size(); i++){
					if (Math.abs(endX-(parent.x+1))+Math.abs(endY-parent.y)+parent.fCost<open.get(i).fCost){
						open.add(i, new Node(parent.x+1,parent.y, parent.x, parent.y, Math.abs(endX-(parent.x+1))+Math.abs(endY-parent.y)+parent.fCost));
						break;
					}else if (i==open.size()-1){
						open.add(new Node(parent.x+1,parent.y, parent.x, parent.y, Math.abs(endX-(parent.x+1))+Math.abs(endY-parent.y)+parent.fCost));
						break;
					}
				}
			}
		}if (map[parent.y][parent.x-1] != 1 && notClosed(parent.x-1,parent.y,Math.abs(endX-(parent.x-1))+Math.abs(endY-parent.y)+parent.fCost,parent.x,parent.y)){
			if (open.size()==0){
				open.add(new Node(parent.x-1,parent.y, parent.x, parent.y, Math.abs(endX-(parent.x-1))+Math.abs(endY-parent.y)+parent.fCost));
			}else if (notQueued(parent.x-1,parent.y)){
				for (int i=0; i<open.size(); i++){
					if (Math.abs(endX-(parent.x-1))+Math.abs(endY-parent.y)+parent.fCost<open.get(i).fCost){
						open.add(i, new Node(parent.x-1,parent.y, parent.x, parent.y, Math.abs(endX-(parent.x-1))+Math.abs(endY-parent.y)+parent.fCost));
						break;
					}else if (i==open.size()-1){
						open.add(new Node(parent.x-1,parent.y, parent.x, parent.y, Math.abs(endX-(parent.x-1))+Math.abs(endY-parent.y)+parent.fCost));
						break;
					}
				}
			}
		}
	}
	
	private boolean notClosed(int x, int y, int fCost, int parentX, int parentY){
		for (int i=0; i<closed.size(); i++){
			if (x == closed.get(i).x && y == closed.get(i).y){
				if (closed.get(i).fCost>fCost){
					closed.get(i).parentX=parentX;
					closed.get(i).parentY=parentY;
				}
				return false;
			}
		}
		return true;
	}
	
	private boolean notQueued(int x, int y){
		for (int i=0; i<open.size(); i++){
			if (x == open.get(i).x && y == open.get(i).y){
				return false;
			}
		}
		return true;
	}
	
	private ArrayList<Node> reconstructPath(){
		ArrayList<Node> path = new ArrayList<Node>();
		
		int currentParentX = closed.get(closed.size()-1).parentX;
		int currentParentY = closed.get(closed.size()-1).parentY;
		
		path.add(closed.get(closed.size()-1));
		
		boolean finished = false;
		
		here:
		while (finished == false){
			for (int i=0; i<closed.size(); i++){
				if (closed.get(i).x == currentParentX && closed.get(i).y == currentParentY){
					path.add(0, closed.get(i));
					if (currentParentX == startX && currentParentY == startY) finished = true;
					
					currentParentX = closed.get(i).parentX;
					currentParentY = closed.get(i).parentY;
					break;
				}else if (i==closed.size()-1){
					System.out.println("ERROR: could not reconstruct path.");
					break here;
				}
			}
		}
		
		return path;
	}

and this is the node class


public class Node {
	public int x, y;
	public int parentX, parentY;
	public int fCost;
	
	public Node(int x, int y, int parentX, int parentY, int fCost){
		this.x = x;
		this.y = y;
		
		this.parentX = parentX;
		this.parentY = parentY;
		
		this.fCost = fCost;
	}
	
	public String toString(){
		return "(" + x + ", " + y + ")";
	}
}

thank you so much in advance… any help would be great! ;D

#2

[quote=“enigma7995,post:1,topic:40058”]
I would start by refactoring it to remove the massive duplication in addAdj and by documenting the meaning of map.

#3

[quote=“pjt33,post:2,topic:40058”]

by refactoring do you mean using an automated code refactorer and the map is essentially the entire world map. Right now i have not implemented a feature to search only up to a certain amount of the map before it quits. I am more worried that i am using bad or slow logic, or that i am just using poor programming procedures to tackle the problem. And thank you for the quick response!

#4

You use the wrong data structures. A* is better implemented with priority queues or a data structure called “heap” (not to be mistaken with the heap which is the main memory of computers).

A* and binary heap tutorial:
http://www.policyalmanac.org/games/binaryHeaps.htm

Refactorings are often done manually or with some help from tools, seldom purely automatic.

#5

For the closed nodes, you can simply use a (hash)set.

#6

No, I mean asking yourself what the differences are between

if (map[parent.y-1][parent.x] != 1 && notClosed(parent.x,parent.y-1,Math.abs(endX-parent.x)+Math.abs(endY-(parent.y-1))+parent.fCost,parent.x,parent.y)){
         if (open.size()==0){
            open.add(new Node(parent.x,parent.y-1, parent.x, parent.y, Math.abs(endX-parent.x)+Math.abs(endY-(parent.y-1))+parent.fCost));
         }else if (notQueued(parent.x,parent.y-1)){
            for (int i=0; i<open.size(); i++){
               if (Math.abs(endX-parent.x)+Math.abs(endY-(parent.y-1))+parent.fCost<open.get(i).fCost){
                  open.add(i, new Node(parent.x,parent.y-1, parent.x, parent.y, Math.abs(endX-parent.x)+Math.abs(endY-(parent.y-1))+parent.fCost));
                  break;
               }else if (i==open.size()-1){
                  open.add(new Node(parent.x,parent.y-1, parent.x, parent.y, Math.abs(endX-parent.x)+Math.abs(endY-(parent.y-1))+parent.fCost));
                  break;
               }
            }
         }
      }

and

if (map[parent.y][parent.x+1] != 1 && notClosed(parent.x+1,parent.y,Math.abs(endX-(parent.x+1))+Math.abs(endY-parent.y)+parent.fCost,parent.x,parent.y) ){
         if (open.size()==0){
            open.add(new Node(parent.x+1,parent.y, parent.x, parent.y, Math.abs(endX-(parent.x+1))+Math.abs(endY-parent.y)+parent.fCost));
         }else if (notQueued(parent.x+1,parent.y)){
            for (int i=0; i<open.size(); i++){
               if (Math.abs(endX-(parent.x+1))+Math.abs(endY-parent.y)+parent.fCost<open.get(i).fCost){
                  open.add(i, new Node(parent.x+1,parent.y, parent.x, parent.y, Math.abs(endX-(parent.x+1))+Math.abs(endY-parent.y)+parent.fCost));
                  break;
               }else if (i==open.size()-1){
                  open.add(new Node(parent.x+1,parent.y, parent.x, parent.y, Math.abs(endX-(parent.x+1))+Math.abs(endY-parent.y)+parent.fCost));
                  break;
               }
            }
         }
      }

and then extracting a method which contains all of the repeated stuff. Your class is about twice as long as it needs to be, which makes it harder to everyone else (and for you in 3 months’ time) to understand it quickly.

[quote=“enigma7995,post:3,topic:40058”]
It’s an int[][]. What does each int mean? The cost of moving into that square? The cost of moving out of that square? An index into an array of terrain properties? A height, with the cost of moving between two squares depending non-linearly on the height difference? (I was assigned a project back in university which used precisely that for the map).

#7

Thank you everyone for your helpful responses. I am going to try and use the suggested priority queues and i will rewrite my addAdj() method. I would also like to apologize for my lack of clarity. All the map is right now is a big array of walkable (0) or non-walkable (1) tiles. There are no tiles that have a greater cost than another. Anyways what type of speed should i expect from this sort of algorithm and how much more performance should i expect out of a professionally written library. I’m semi new to game development and I am young in general so i really appreciate your time and understanding.

#8

On a somewhat modern computer you should expect pathes of less than 200 tiles to be found in a few milliseconds. So unless you have thousands of units doing pathfinding, it should be really quick.