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

Flash memory saving optimisations #52

Merged
merged 3 commits into from Mar 24, 2014
Merged

Flash memory saving optimisations #52

merged 3 commits into from Mar 24, 2014

Conversation

@ribbons
Copy link
Contributor

@ribbons ribbons commented Mar 23, 2014

These commits reduce flash memory usage by the library (by 426 bytes in my current project) by optimising the variables slightly, only storing strings that are used in flash and removing repetition of the server header value.

ribbons added 2 commits Mar 23, 2014
Trim down the size of a couple of variables as they didn't need to be as
large, clarify the type of some others, and change port from int to
uint16_t to match EthernetServer.
Change the definition of the P() macro under the AVR architecture to store
the strings in sections named after the variables.  This allows unused
strings to be removed by the linker, saving space in the flash memory.

Also change the variable name for all but one of the instances of failMsg
to give the maximum benefit from the above.
@@ -160,6 +154,8 @@ enum URLPARAM_RESULT { URLPARAM_OK,
URLPARAM_EOS // No params left
};

P(webServerHeader) = "Server: Webduino/" WEBDUINO_VERSION_STRING CRLF;
Copy link
Collaborator

@unwiredben unwiredben Mar 24, 2014

Choose a reason for hiding this comment

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

Shouldn't you guard this definition with WEBDUINO_SUPRESS_SERVER_HEADER?

Copy link
Contributor Author

@ribbons ribbons Mar 24, 2014

Choose a reason for hiding this comment

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

Shouldn't you guard this definition with WEBDUINO_SUPRESS_SERVER_HEADER?

I did originally, but checked that once the string isn't referenced, the section containing it is removed by the linker (due to f95122d in this PR on AVR), so it doesn't use any space in flash.

Would you prefer me to wrap it with an #ifndef WEBDUINO_SUPRESS_SERVER_HEADER anyway for clarity?

Copy link
Collaborator

@unwiredben unwiredben Mar 24, 2014

Choose a reason for hiding this comment

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

Actually, the main guard I want is #ifndef WEBDUINO_NO_IMPLEMENTATION which turns off definitions in the file... this lets a multi-file sketch include the .h file in multiple places, but only have one where it creates definitions. Your explanation of how the linker drops the section satisifies my other concern.

Modify the functions which send headers to re-use one string containing
the server name instead of including it inline in all of the different
headers.
@ribbons
Copy link
Contributor Author

@ribbons ribbons commented Mar 24, 2014

Have updated this PR to move the definition of webServerHeader within the #ifndef WEBDUINO_NO_IMPLEMENTATION to address the issue pointed out by @unwiredben.

unwiredben added a commit that referenced this issue Mar 24, 2014
Flash memory saving optimisations
@unwiredben unwiredben merged commit 0b1463d into sirleech:master Mar 24, 2014
@unwiredben
Copy link
Collaborator

@unwiredben unwiredben commented Mar 24, 2014

Merged in, thanks!

@ribbons ribbons deleted the flash-saving branch Mar 24, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants