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

Convert to PROGMEM #3

Merged
merged 1 commit into from
Dec 20, 2021
Merged

Convert to PROGMEM #3

merged 1 commit into from
Dec 20, 2021

Conversation

drashna
Copy link
Contributor

@drashna drashna commented Dec 13, 2021

This converts the array to PROGMEM, in theory, and should reduce system memory usage on AVR boards. ARM doesn't use progmem, but has a lot more memory, so shouldn't be an issue

@getreuer
Copy link
Owner

Thank you @drashna ! This is great. I verified that this works on Dactyl Ergodox.

I was at first concerned about the overhead of reading from PROGMEM, since the code reads some number of bytes from PROGMEM on every key press, and in a (cache unfriendly) jumping pattern to follow trie branches. So I did some benchmarking to check. Method: on every alpha key press event, I used QMK's timer_read32() and timer_elapsed32() around the process_autocorrection() call. Since this has only millisecond resolution, I accumulate the elapsed time across multiple key events to get a sub-millisecond estimate the average. For normal typing, estimated over a few thousand keystrokes on Monkeytype exercises, I get:

Version Average time
Without PROGMEM 26 μs
With PROGMEM 20 μs

That's comparable if not better!

In order to merge this, I need you to sign a Contributor License Agreement (CLA) at https://cla.developers.google.com/. You (or your employer) retain the copyright to your contribution. This simply gives permission to use and redistribute your contributions as part of the project. See https://cla.developers.google.com/ for your current agreements on file or to sign a new one.

@drashna
Copy link
Contributor Author

drashna commented Dec 19, 2021

I'm glad to hear that it seems to be even faster! Though, my primary concern was for memory usage on AVR, which can be very limited. For ARM, it doesn't use progmem, at all, but I'm used to keeping both in mind.

And the CLA "signed".

@getreuer getreuer merged commit 7b672ed into getreuer:main Dec 20, 2021
@drashna drashna deleted the progmem branch December 21, 2021 17:10
fride added a commit to fride/qmk-keymap that referenced this pull request Aug 5, 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.

2 participants