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

reorder moves for better cache locality #300

Closed
wants to merge 1 commit into from
Closed

Conversation

leijurv
Copy link
Member

@leijurv leijurv commented Dec 26, 2018

need to merge onto benchmark and, uh, benchmark, this should bump up the percent of BlockStateInterface lookups that are in the same cached chunk / region as the previous by a couple percent, i anticipate. it won't actually increase that metric in benchmark, because every movement gets data from both the source chunk and the destination chunk, but it will improve the cache locality of the adjacent chunks by going on counterclockwise order like this (instead of random). or at least i hope it will.

@leijurv
Copy link
Member Author

leijurv commented Dec 26, 2018

the diff looks absolutely destroyed, but rest assured i just rearranged the enum values, and didn't change any of their contents

@leijurv
Copy link
Member Author

leijurv commented Jan 11, 2019

Note to future self: do NOT just merge this without actually benching it lol

I'm not 100% certain it's better, there is also cache locality in the vertical ExtendedBlockStorages, and recall that over 90% of the time lookups are in the same chunk. Therefore vertical locality might be more important.

@leijurv
Copy link
Member Author

leijurv commented Jan 17, 2019

Benched this and it was slower =(

Methodology is as normal:

Have baritone with all default settings except timeout extended to 4 seconds, stand at x=1368, y=68, z=457 in the auto test seed, path towards 0,0.

Repeat, keeping track of the node counts. Wait until the first time the node count is less than previous, start counting the previous (so this second run is always lower node count than the first), so that the JIT can warm up and stuff. Average over 10 runs.

Master was 404544,403776,428416,404544,377152,372736,363776,398336,397888,396608,416000, for an average of 398706.

This branch was 441280,401216,384128,396608,379648,387072,334080,377024,392896,348160,361664, for an average of 382161.

I then ran master again because I didn't trust this, and got 408384,406976,383744,421184,497088,408064,393856,424960,420800,430144,430016, for an average of 420474.

And that run also set a record on the 5th run, lol

[12:58:25] [main/INFO]: [CHAT] §5[§dBaritone§5]§7 Starting to search for path from BlockPos{x=1368, y=68, z=457} to GoalXZ{x=0,z=0}
[12:58:29] [pool-4-thread-4/INFO]: [STDOUT]: 10935936 movements considered
[12:58:29] [pool-4-thread-4/INFO]: [STDOUT]: Open set size: 124512
[12:58:29] [pool-4-thread-4/INFO]: [STDOUT]: PathNode map size: 621600
[12:58:29] [pool-4-thread-4/INFO]: [STDOUT]: 124272 nodes per second
[12:58:29] [pool-4-thread-4/INFO]: [STDOUT]: Path goes for 245.63387388550464 blocks
[12:58:29] [main/INFO]: [CHAT] §5[§dBaritone§5]§7 Took 4000ms, A* cost coefficient 1.5, 10935936 movements considered
[12:58:29] [main/INFO]: [CHAT] §5[§dBaritone§5]§7 Static cutoff 287 to 261
[12:58:29] [main/INFO]: [CHAT] §5[§dBaritone§5]§7 Found path segment from BlockPos{x=1368, y=68, z=457} towards GoalXZ{x=0,z=0}. 497088 nodes considered

This is the best I've ever gotten on my laptop. 10.9 million movements considered in 4 seconds!

So master is faster =(

@leijurv leijurv closed this Jan 17, 2019
@leijurv leijurv deleted the moves-reorder branch January 17, 2019 21:02
@leijurv
Copy link
Member Author

leijurv commented Jan 17, 2019

Speed comparison to minebot is linked in the above pr (#180)

Baritone is now 32.7x faster.

that bittersweet moment when you're benching a pr that you think will make performance epic but it actually makes it slower and master actually sets a new speed record

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant