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

🚧 Planner flag optimizations #4433

Open
wants to merge 2 commits into
base: MK3
Choose a base branch
from

Conversation

leptun
Copy link
Collaborator

@leptun leptun commented Oct 8, 2023

Combined boolean variables in the planner struct with the "flag" variable

@leptun leptun requested a review from gudnimg October 8, 2023 10:10
@github-actions
Copy link

github-actions bot commented Oct 8, 2023

All values in bytes. Δ Delta to base

Target ΔFlash ΔSRAM Used Flash Used SRAM Free Flash Free SRAM
MK3S_MULTILANG -58 -32 245924 5601 8028 2591
MK3_MULTILANG -58 -32 245220 5610 8732 2582

@@ -233,7 +235,7 @@ FORCE_INLINE block_t *plan_get_current_block()
return(NULL);
}
block_t *block = &block_buffer[block_buffer_tail];
block->busy = true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Note this is called in the Stepper ISR. Is it safe to drop the volatile keyword?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I worry about race conditions here

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think yes. I didn't see any place where volatile was really needed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Added @wavexx as reviewer, I think he would know best if this is OK.

@gudnimg gudnimg requested a review from wavexx November 5, 2023 12:58
@leptun leptun added this to the FW 3.14.1 milestone Nov 25, 2023
@3d-gussner
Copy link
Collaborator

@leptun @gudnimg Any update on this one?

@3d-gussner 3d-gussner removed this from the FW 3.14.1 milestone Apr 30, 2024
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