Skip to content

Memory savings by replacing strings#792

Merged
JF002 merged 1 commit intoInfiniTimeOrg:developfrom
Riksu9000:memory_saving
Nov 6, 2021
Merged

Memory savings by replacing strings#792
JF002 merged 1 commit intoInfiniTimeOrg:developfrom
Riksu9000:memory_saving

Conversation

@Riksu9000
Copy link
Contributor

@Riksu9000 Riksu9000 commented Oct 29, 2021

Saves 996 bytes!

@lman0 lman0 mentioned this pull request Oct 29, 2021
Closed
@JF002
Copy link
Collaborator

JF002 commented Oct 30, 2021

@Riksu9000 Impressive! Does it free RAM or FLASH memory?

@JF002 JF002 added this to the 1.7.0 milestone Oct 30, 2021
@Riksu9000
Copy link
Contributor Author

Riksu9000 commented Oct 30, 2021

@Riksu9000 Impressive! Does it free RAM or FLASH memory?

These changes only free FLASH memory.

@JF002
Copy link
Collaborator

JF002 commented Oct 30, 2021

How come those std::to_string were using so much space in flash?

@geekbozu
Copy link
Member

geekbozu commented Oct 31, 2021 •
edited
Loading

there are still a few more instances of std:to_string left in the repo it seems. In the date time library. I wonder if removing those will yield some more space? I'm confused as well as to why this frees up so much room. Is it just because we no longer link to_string in the infinitime blob? Or is that just that hungry of a routine...in flash?

A quick skim makes it seem like we don't use the date library those other calls are in. Which would explain the flash savings by just out-rite no longer including the routine! Well worth the change I think.

@JF002 JF002 merged commit cfc11ea into InfiniTimeOrg:develop Nov 6, 2021
@Riksu9000 Riksu9000 deleted the memory_saving branch November 6, 2021 10:12
@Riksu9000 Riksu9000 mentioned this pull request Dec 3, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Avamander Avamander Avamander approved these changes

Assignees

No one assigned

Labels

hacktoberfest-accepted

Projects

None yet

Milestone

1.7.0

Development

Successfully merging this pull request may close these issues.

4 participants

@Riksu9000 @JF002 @geekbozu @Avamander