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

iostream support blows up binary size and RAM requirements #32

Closed
pvaibhav opened this issue Nov 3, 2016 · 3 comments
Closed

iostream support blows up binary size and RAM requirements #32

pvaibhav opened this issue Nov 3, 2016 · 3 comments
Assignees
Milestone

Comments

@pvaibhav
Copy link
Contributor

pvaibhav commented Nov 3, 2016

Currently iostream support is unconditionally enabled. This is fine for desktop targets but not for small embedded targets with limited flash storage and RAM. For instance on the Nordic nRF51 based on ARM Cortex-M0+, approx 150kB flash and 6.5kB of extra RAM is used, on a device which only has 256kB flash and 8kB RAM.

Disabling iostream support will go a long way in supporting embedded targets.

The following patch (against v2.2.x branch) adds a #define to enable/disable iostream:
https://gist.github.com/pvaibhav/c3703bf23c8513f4a8160eed524a415d

I've verified disabling iostream results in zero impact on the compiled binary size or RAM usage.

I'll create a pull request from this patch if there is any interest. My only concern is the duplication of UNIT_ADD definition, once with and once without iostream. An intermediate macro could perhaps help here.

pvaibhav added a commit to TobyRich/units that referenced this issue Nov 3, 2016
pvaibhav added a commit to TobyRich/units that referenced this issue Nov 3, 2016
@nholthaus nholthaus self-assigned this Nov 3, 2016
@nholthaus nholthaus added this to the v2.2.0 milestone Nov 3, 2016
@nholthaus
Copy link
Owner

nholthaus commented Nov 3, 2016

I'm all about supporting embedded. I'd tacitly assumed that the template meta-programming would blow up small microprocessors, so I originally wasn't worried about iostream or includes in general. If that's not (always) the case, then I'm definitely interested.


In terms of the implementation I think I'd prefer to see UNITS_LIB_ENABLE_IOSTREAM be UNITS_LIB_DISABLE_IOSTREAM, and have it not be defined by default in the library, that way it can be defined in user code (or on the cmake command line) rather than having to patch the library for embedded use.

I also agree about the macros. I think they need to be refactored such that UNITS_ADD becomes a macro-of-macros depending on which features are enabled (iostream/literals/etc). This would also allow the duplication in the VS 2013 macros to be removed (probably by defining UNITS_LIB_DISABLE_LITERALS or something).

I can work on this and get it into v2.2, unless you'd prefer to make the changes.

@pvaibhav
Copy link
Contributor Author

pvaibhav commented Nov 3, 2016

No problem, I'll refactor a bit based on your suggestions and send a patch tomorrow!

@nholthaus
Copy link
Owner

This is being tackled in PR #42 now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants