New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Increase PathFinder maxRooms maximum to 64 #31

Merged
merged 1 commit into from Dec 10, 2018

Conversation

Projects
None yet
2 participants
@laverdet
Copy link
Contributor

laverdet commented Nov 8, 2018

Some users have said that increasing this limit would be useful to them. Since any time the pathfinder enters a room it counts against maxRooms, you usually end up with a maximum path shorter than 16 rooms long.

With the existing code the highest we could bump the k_max_rooms constant to was 24, so I went through and audited and updated the data structures in use. This increases memory usage a little, but reduces CPU overhead a little bit too (by about 7%). The pathfinder is almost allocation-free which means that it allocates all the memory it could possibly need upfront and holds on to it for the program's lifetime. The old code would use ~49kb of memory per thread per k_max_rooms, plus about 74kb of other overhead. The new code uses ~84kb of memory per thread per k_max_rooms, plus the same overhead.

Assuming a 4 thread server, before this patch the pathfinder will hold onto ~3.63mb of memory. After this patch it will hold onto 17.27mb of memory. I don't think that number is unreasonable by any measure but I did want to make sure to communicate the exact cost of this change. And like I mentioned it is 7% faster, at least on my system and probably most 64-bit systems.

This also includes the fix for the "Max heap" issue we discussed a couple days ago. It'll automatically adjust the heap size to the correct capacity based on k_max_rooms.

@artch

This comment has been minimized.

Copy link
Contributor

artch commented Nov 8, 2018

Looks good, thanks! It will be merged into the next PTR patch. Could you please rebase it onto ptr branch? Especially because it already contains the fix in e897bda commit.

@artch artch added the ptr-staging label Nov 8, 2018

@laverdet laverdet force-pushed the laverdet:pf branch from 8edec74 to 75abbe5 Nov 8, 2018

@laverdet laverdet changed the base branch from master to ptr Nov 8, 2018

@artch artch merged commit 75abbe5 into screeps:ptr Dec 10, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment