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

Allow the trace output by mbed error to be conditional of NDEBUG. #2715

Merged
merged 1 commit into from Sep 24, 2016

Conversation

Projects
None yet
3 participants
@pan-
Member

pan- commented Sep 15, 2016

This change disable any trace output by error when the application is compiled with NDEBUG.

Unlike assert, error will still crash/halt the execution of the application even if NDEBUG is enabled.
In that regard, its core behavior is preserved.

This change avoid the inclusion of printf and friends code in a binary when it is compiled with the macro NDEBUG enabled because a lot of driver code use error rather than assert.

The memory gain is non negligible: This is the differences with and without this change when mbed-os-example-blinky is compilef with NDEBUG enabled.

  • Without the patch:
GCC ARMCC IAR
RAM 11832 10152 7970
ROM 37008 26868 21266
  • With the patch:
GCC ARMCC IAR
RAM 11808 10152 7834
ROM 19852 20239 14600
  • Difference
GCC ARMCC IAR
RAM -24 bytes (-0.2%) 0 bytes (0%) -136 bytes (-1.70%)
ROM -17156 bytes (-46.3%) -6629 bytes (-24.7%) ROM: -6666 bytes (-31.3%)
Allow the trace output by mbed error to be conditional of NDEBUG.
This change avoid inclusion of printf and friends code in a binary when it
is compiled with the macro NDEBUG enabled (this macro is usually enabled
for production builds).

Unlike assert, the error function will still crash/halt the execution of the
application even if NDEBUG is enabled; the traces are just not outputed.
@pan-

This comment has been minimized.

Show comment
Hide comment
@pan-

pan- Sep 15, 2016

Member

@0xc0170 @sg- Waiting for your comments.
I would like to know where the documentation around NDEBUG should be located.
For example, MBED_ASSERT use it but it is not documented.

Member

pan- commented Sep 15, 2016

@0xc0170 @sg- Waiting for your comments.
I would like to know where the documentation around NDEBUG should be located.
For example, MBED_ASSERT use it but it is not documented.

@sg-

This comment has been minimized.

Show comment
Hide comment
@sg-

sg- Sep 16, 2016

Member

I would like to know where the documentation around NDEBUG should be located.

I'd suggest we have 2, maybe 3 max build profiles that compliment this patch and have specific objectives and flags / macros used based on that. The never ending list of -o options should be wrapped into build profiles and documented. Something like -o size, speed, debug.

Member

sg- commented Sep 16, 2016

I would like to know where the documentation around NDEBUG should be located.

I'd suggest we have 2, maybe 3 max build profiles that compliment this patch and have specific objectives and flags / macros used based on that. The never ending list of -o options should be wrapped into build profiles and documented. Something like -o size, speed, debug.

@0xc0170

This comment has been minimized.

Show comment
Hide comment
@0xc0170

0xc0170 Sep 20, 2016

Member

The never ending list of -o options should be wrapped into build profiles and documented

+1 for having profiles

This change-set looks good to me. We will discuss connection between NDEBUG and profiles once profiles get into the codebase, or?

@pan- 👍 for this

Member

0xc0170 commented Sep 20, 2016

The never ending list of -o options should be wrapped into build profiles and documented

+1 for having profiles

This change-set looks good to me. We will discuss connection between NDEBUG and profiles once profiles get into the codebase, or?

@pan- 👍 for this

@sg- sg- added ready for merge and removed needs: review labels Sep 22, 2016

@sg- sg- merged commit bd3d6ab into ARMmbed:master Sep 24, 2016

9 of 12 checks passed

Cambridge CI mbed-os_uvisor_profile_0 Something went wrong. Investigate! No test results found.
Details
Cambridge CI mbed-os_uvisor_profile_1 Something went wrong. Investigate! No test results found.
Details
continuous-integration/jenkins/pr-merge This commit is being built
Details
Cambridge CI mbed-os_uvisor_profile_2 All is well No test results found.
Details
Cambridge CI mbed-os_uvisor_profile_3 All is well No test results found.
Details
Jenkins K64F-ARM Build:Passed Tests:Passed Build #2082 succeeded in 14 min
Details
Jenkins K64F-GCC_ARM Build:Passed Tests:Passed Build #2083 succeeded in 14 min
Details
Jenkins K64F-IAR Build:Passed Tests:Passed Build #2084 succeeded in 14 min
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
mbed-os CI Checks Build finished.
Details
uvisor K64F-GCC_ARM-uvisor_disabled All is well still in progress... No test results found.
Details
uvisor K64F-GCC_ARM-uvisor_enabled All is well still in progress... No test results found.
Details

pan- added a commit to pan-/mbed that referenced this pull request Oct 21, 2016

@pan- pan- deleted the pan-:NDEBUG_optimization branch Jul 3, 2018

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