use less stack on Print::printNumber #528
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
DEvil0000
force-pushed
the
use-less-stack-on-number-print
branch
2 times, most recently
from
1a61a90
to
8e67a64
Compare
|
Memory usage change @ 8e67a64
Click for full report table
Click for full report CSV |
|
Hi @DEvil0000. Thanks for your pull request. I'm just commenting to let you know the failure of the "Spell Check" workflow run is not related to the changes you proposed (it was caused by a recent automated update of the spellcheck dictionary) and won't affect the review of the PR. I have now fixed the problem in the |
|
This PR causes digits to print in reverse order. Tested with Arduino Uno using IDE 1.8.19 with AVR core 1.8.6.
|
added compile flag to disable those additional optimized implementations
DEvil0000
force-pushed
the
use-less-stack-on-number-print
branch
from
8e67a64
to
870432e
Compare
|
@PaulStoffregen sorry, looks like I was a bit sleepy when opening the PR. I fixed the implementation with the first two commits and added improved versions for base 2 and base 16 which one can disable by compiler define flag in the third commit. |
|
Memory usage change @ 870432e
Click for full report table
Click for full report CSV |
|
Memory usage change @ fd90b45
Click for full report table
Click for full report CSV |
|
Memory usage change @ 66645a2
Click for full report table
Click for full report CSV |
|
Memory usage change @ e5c727e
Click for full report table
Click for full report CSV |
… optimization due to it being basically const then
|
Memory usage change @ a81294f
Click for full report table
Click for full report CSV |
|
Yeah, catering for all the bases with one function messes it up there. When printing decimal only, much smaller buffer is needed(11bytes vs 33bytes). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The issue with printNumber is that it uses a lot of stack quite unexpectedly. This makes it basically unusable on a attiny.
I suggest doing the naive implementation as it's done with float print.
I think the Autor was hoping for a more optimized write call Implementation in the over-writable write call with a long buffer. I think this is too late to get it more efficient for most platforms anyway. Another option of course would be changing the API and adding a parameter for this or adding a define to switch the default Implementation.