Skip to content
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

path caching #1595

Merged
merged 4 commits into from
Nov 14, 2023
Merged

path caching #1595

merged 4 commits into from
Nov 14, 2023

Conversation

kostmo
Copy link
Member

@kostmo kostmo commented Oct 21, 2023

Closes #1569

Performance

Path cache invalidation upon world modifications (i.e. entities inserted or removed) entails iterating over all of the previously-cached paths here. For efficiency's sake, we avoid iterating over "all existing robots".

Any scenario that does not use the path command is entirely unaffected by this change.

Demo

Previously, this demo was virtually unplayable, since when moving between widely-spaced "clusters" of flowers, an expensive A-star search was invoked at almost every tick. Now, the vast majority of moves utilize the cache, and the demo exhibits minimal stuttering (e.g. when a single A-star search is performed when moving between distant clusters).

scripts/play.sh -i scenarios/Fun/horton.yaml --autoplay --speed 7

Event log

An event log specific to the path cache is maintained with its own ring buffer:

scripts/play.sh -i scenarios/Testing/1569-pathfinding-cache/1569-harvest-batch.yaml --autoplay

and view http://localhost:5357/paths/log

@restyled-io restyled-io bot mentioned this pull request Oct 21, 2023
@kostmo kostmo changed the base branch from main to feature/structure-previews October 22, 2023 02:14
@kostmo kostmo force-pushed the feature/path-caching branch from 94d9c7b to ceae4ef Compare October 22, 2023 02:14
@kostmo kostmo mentioned this pull request Oct 22, 2023
@kostmo kostmo force-pushed the feature/path-caching branch 4 times, most recently from 4b976fb to 800ce11 Compare October 23, 2023 00:28
@kostmo kostmo force-pushed the feature/structure-previews branch from be197fe to 97fe6b0 Compare October 23, 2023 05:34
@kostmo kostmo force-pushed the feature/path-caching branch from 800ce11 to c5661c0 Compare October 23, 2023 05:35
@kostmo kostmo force-pushed the feature/structure-previews branch from 97fe6b0 to 23b914d Compare October 23, 2023 05:49
@kostmo kostmo force-pushed the feature/structure-previews branch from 23b914d to 2fc6257 Compare October 29, 2023 02:00
@kostmo kostmo changed the base branch from feature/structure-previews to refactor/extract-entity-modification October 29, 2023 02:09
@kostmo kostmo force-pushed the feature/path-caching branch from c5661c0 to 2a571ca Compare October 29, 2023 02:23
mergify bot pushed a commit that referenced this pull request Oct 29, 2023
This is a refactoring that is a prerequisite for both #1579 and #1595.
@kostmo kostmo force-pushed the feature/path-caching branch from 2a571ca to 86de12b Compare October 29, 2023 22:34
@kostmo kostmo added the CHANGELOG Once merged, this PR should be highlighted in the changelog for the next release. label Nov 9, 2023
@kostmo kostmo changed the base branch from refactor/extract-entity-modification to main November 10, 2023 04:30
@kostmo kostmo changed the base branch from main to refactor/walkability November 10, 2023 04:57
@kostmo kostmo force-pushed the feature/path-caching branch from 86de12b to 195f81e Compare November 10, 2023 04:57
@kostmo kostmo force-pushed the refactor/walkability branch 2 times, most recently from 47ba7ff to 05f7624 Compare November 10, 2023 05:49
@kostmo kostmo force-pushed the feature/path-caching branch 2 times, most recently from 3d121a5 to 2442db2 Compare November 10, 2023 06:29
mergify bot pushed a commit that referenced this pull request Nov 10, 2023
This simplifies the implementation of #1595.
@kostmo kostmo changed the base branch from refactor/walkability to main November 10, 2023 22:27
@kostmo kostmo force-pushed the feature/path-caching branch 2 times, most recently from 88bb8af to 709bda4 Compare November 11, 2023 02:51
@kostmo kostmo force-pushed the feature/path-caching branch 2 times, most recently from 6812c1e to 1a1d577 Compare November 11, 2023 06:34
@restyled-io restyled-io bot mentioned this pull request Nov 11, 2023
@kostmo kostmo force-pushed the feature/path-caching branch from 1a1d577 to 2ffe099 Compare November 11, 2023 06:34
@@ -74,6 +72,9 @@ updateEntityAt cLoc@(Cosmic subworldName loc) upd = do
wakeWatchingRobots cLoc
SRT.entityModified modType cLoc

pcr <- use $ pathCaching . pathCachingRobots
mapM_ (revalidatePathCache cLoc modType) $ IM.toList pcr
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cache invalidation callback is here.

@kostmo kostmo force-pushed the feature/path-caching branch from 2ffe099 to 8df24b5 Compare November 11, 2023 07:10
@kostmo kostmo added L-Commands Built-in commands (e.g. move, try, if, ...) in the Swarm language. Z-Performance This issue concerns memory or time efficiency of the Swarm game. labels Nov 11, 2023
@kostmo kostmo force-pushed the feature/path-caching branch from 8df24b5 to 22a1a39 Compare November 11, 2023 07:41
@kostmo kostmo marked this pull request as ready for review November 11, 2023 07:41
@kostmo kostmo requested review from byorgey and xsebek November 11, 2023 07:41
Copy link
Member

@byorgey byorgey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kostmo , thanks so much for the detailed comments and the amazing attention to detail with the whole cache logging system + tests!

I must admit I was skeptical of the idea of shortest path caching initially --- I was worried about performance, etc. --- but (1) the Horton's flower scenario is fun, and is a convincing demonstration of the utility of path caching, and (2) I am convinced that this will have minimal/zero impact whenever the path command is not being used.

I would love to eventually see a series of challenges introducing the path command (not saying you have to develop them).

data/scenarios/Fun/horton.yaml Outdated Show resolved Hide resolved
src/Swarm/Game/Step/Path/Cache.hs Outdated Show resolved Hide resolved
src/Swarm/Game/Step/Path/Finding.hs Outdated Show resolved Hide resolved
src/Swarm/Game/Step/Path/Finding.hs Outdated Show resolved Hide resolved
kostmo and others added 3 commits November 14, 2023 11:14
@kostmo kostmo force-pushed the feature/path-caching branch from 02d36d3 to de162cc Compare November 14, 2023 20:44
@kostmo kostmo added the merge me Trigger the merge process of the Pull request. label Nov 14, 2023
@mergify mergify bot merged commit f9ef009 into main Nov 14, 2023
9 checks passed
@mergify mergify bot deleted the feature/path-caching branch November 14, 2023 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CHANGELOG Once merged, this PR should be highlighted in the changelog for the next release. L-Commands Built-in commands (e.g. move, try, if, ...) in the Swarm language. merge me Trigger the merge process of the Pull request. Z-Performance This issue concerns memory or time efficiency of the Swarm game.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Caching for pathfinding
2 participants