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(perf): Use fork of bevy_simple_tilemap with rayon par_iter feature disabled until PR merged. #133

Merged
merged 1 commit into from
Jul 29, 2023

Conversation

MaxCWhitehead
Copy link
Collaborator

This fixes fishfolk/jumpy#841, can switch back to original crate once fix is merged upstream of fork.

Change we are introducing is here: forbjok/bevy_simple_tilemap@master...MaxCWhitehead:bevy_simple_tilemap:v0.11.0
Gates rayon behind feature flag, which we do not use.

@MaxCWhitehead
Copy link
Collaborator Author

@zicklag might have made a mess of this PR haha, I fixed the bevy version having been 0.10.0 in fork vs 0.10.1, should fix the previous minimal dependencies error. I couldn't figure out how to rerun the github actions, perhaps I don't have permission?

So I pushed an empty commit to trigger, then I was going to force-push it out before merging, so I disabled auto-merge. It seems actions aren't running though. Going to remove that empty commit, and perhaps you can re-enable auto-merge and re-run actions.

@zicklag
Copy link
Member

zicklag commented Jul 29, 2023

Ah, yeah, it's because it's your first contribution to bones it makes me manually approve any changes you make before CI will run. I'll re-approve and re-auto merge.

@zicklag
Copy link
Member

zicklag commented Jul 29, 2023

Actually it looks like there's another issue with CI here. I'll look at it real quick.

@MaxCWhitehead
Copy link
Collaborator Author

MaxCWhitehead commented Jul 29, 2023

Hmm, appears to be the same one. I tried to run minimal version check command locally and did not reproduce the same error, not sure why exactly, thought that may have been the issue, but guess not. Might have to mess with it more. Let me know if you have any ideas on what commands / tools to use locally to diagnose this type of thing.

@zicklag
Copy link
Member

zicklag commented Jul 29, 2023

It has something to do with the Rust version being different for those two CI jobs, because it has to use nightly Rust to enable certain unstable flags for miri and the minimal deps check.

This isn't the fault of your PR, so I'll just override the the CI job to let this merge.

@MaxCWhitehead
Copy link
Collaborator Author

Ah ok, gotcha.

@zicklag zicklag disabled auto-merge July 29, 2023 21:14
@zicklag zicklag merged commit 1ed71c6 into fishfolk:main Jul 29, 2023
16 of 20 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.

Performance: bevy_simple_tilemap uses Rayon parallel iterators causing substantial performance issues
2 participants