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 comment parsing speed regression #2701

Closed

Conversation

kisslorand
Copy link
Contributor

@kisslorand kisslorand commented Feb 5, 2023

Requirements

BTT or MKS TFT

Description

PR #2671 promised to "Optimize comment parsing in comment.c", instead it crowded the MCU with unnecessary instructions, all these during printing, where speed is crucial. The opposite of optimize was done, there is no benefit from it, only loss. The resulting code is not even smaller with one single byte.
Because of PR #2671 the first word in every comment from the G-code file is preprocessed even if it is not even a candidate for a known keyword. This takes time and it is unnecessary, not to mention again that this is during printing.

This PR brings back the systematic approach that was done before this "optimization" and also does what PR #2671 assumably wanted to do, reduces the footprint of "comment.c" on the FW's size.

An alternative for `strlwr()' and 'strupr()' is introduced. The introduced functions are lighter and are returning the pointer to the processed string.

Benefits

  • faster overall processing of G-code comments

Related Issues

  • luckily none reported yet

@kisslorand kisslorand force-pushed the Fix-speed-regression branch 2 times, most recently from af14ab1 to b1faf80 Compare February 15, 2023 12:08
@kisslorand kisslorand force-pushed the Fix-speed-regression branch from b1faf80 to e032615 Compare March 23, 2023 04:53
@kisslorand kisslorand force-pushed the Fix-speed-regression branch 3 times, most recently from 79a013f to bfd64b0 Compare April 17, 2023 12:50
@kisslorand kisslorand force-pushed the Fix-speed-regression branch from bfd64b0 to f16a412 Compare April 19, 2023 03:55
@kisslorand kisslorand force-pushed the Fix-speed-regression branch from f16a412 to 806853d Compare May 2, 2023 08:57
@kisslorand kisslorand force-pushed the Fix-speed-regression branch from 7f0fcff to b93491d Compare May 24, 2023 17:19
@kisslorand kisslorand force-pushed the Fix-speed-regression branch from b93491d to d15f717 Compare June 5, 2023 06:21
@stale
Copy link

stale bot commented Aug 7, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Abandoned label Aug 7, 2023
@kisslorand
Copy link
Contributor Author

Bump

@stale stale bot removed the Abandoned label Aug 7, 2023
@stale
Copy link

stale bot commented Oct 15, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the Abandoned label Oct 15, 2023
@kisslorand kisslorand force-pushed the Fix-speed-regression branch from d15f717 to f54bd85 Compare October 15, 2023 09:31
@stale stale bot removed the Abandoned label Oct 15, 2023
@kisslorand kisslorand force-pushed the Fix-speed-regression branch 11 times, most recently from ee6c6e0 to 009adc0 Compare November 6, 2023 17:51
@kisslorand kisslorand force-pushed the Fix-speed-regression branch from 009adc0 to 7f3189c Compare November 7, 2023 05:07
@kisslorand kisslorand force-pushed the Fix-speed-regression branch from 7f3189c to 7629777 Compare November 7, 2023 19:16
Copy link

This PR has been automatically marked as stale because it has had no activity for the last 60 days. It will be closed in 7 days if no further activity occurs. Thank you for your contribution.

@github-actions github-actions bot added the Stale label Apr 21, 2024
@github-actions github-actions bot closed this Apr 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant