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

Fix chunk loading #378

Merged
merged 2 commits into from
Dec 7, 2024
Merged

Fix chunk loading #378

merged 2 commits into from
Dec 7, 2024

Conversation

kralverde
Copy link
Contributor

@kralverde kralverde commented Dec 5, 2024

Description

This PR fixes the "but no one was watching spam", removes the need to wait for chunks when removing a player and removes the concept of pending tasks from the player, adds perma-loaded spawn chunks, drastically speeds up loading screen time when joining, and leverages rayon rather than async to stream chunks.

Testing

Joined a world

Checklist

Things need to be done before this Pull Request can be merged.

  • Code is well-formatted and adheres to project style guidelines: cargo fmt
  • Code does not produce any clippy warnings: cargo clippy
  • All unit tests pass: cargo test

@OfficialKris
Copy link
Contributor

Works great. There seems to be a memory leak somewhere because I got up to 700MB ram usage after roaming around at 32 chunks then going back to spawn and turning the setting back to 10. It continued to increase whenever generating chunks and didn't go down. Current master maxes at ~300MB then immediately drops to 51MB after not generating chunks. This was with release build.

@kralverde
Copy link
Contributor Author

kralverde commented Dec 5, 2024

Works great. There seems to be a memory leak somewhere because I got up to 700MB ram usage after roaming around at 32 chunks then going back to spawn and turning the setting back to 10. It continued to increase whenever generating chunks and didn't go down. Current master maxes at ~300MB then immediately drops to 51MB after not generating chunks. This was with release build.

I'm unable to reproduce. Are you sure it's a memory leak and not rust just allocating memory for the internal map?

@kralverde
Copy link
Contributor Author

kralverde commented Dec 7, 2024

@OfficialKris can you check your benchmarks again? I'm curious how this latest commit affects it. I'm getting ~100M of memory usage.

@Snowiiii
Copy link
Owner

Snowiiii commented Dec 7, 2024

Just tested and it works pretty nice. Finally pumpkin is usable in debug builds :D.
Thanks @kralverde

@Snowiiii Snowiiii merged commit cd6ac14 into Snowiiii:master Dec 7, 2024
9 checks passed
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.

3 participants