So how much was the size reduction? thanks, aditi
> On Apr 5, 2016, at 12:41 PM, Christopher Collins <[email protected]> wrote: > > Hello all, > > Yesterday I made a fairly large commit to the net/nimble/host package in the > core repository (the host-side of the BLE stack). I wanted to provide the > community with an explanation of these changes and related changes planned for > the future. This email will probably be long and detailed, so don't > feel bad about skimming it for anything that you find interesting. That > said, I would love to hear any and all feedback. > > First, here is the relevant commit: > Repo: http://git-wip-us.apache.org/repos/asf/incubator-mynewt-core > Branch: develop > Commit: 2fef21d6a1f9d3d83b88ecf11b825a6e5a354a47 > > Motivation for this change: the code size of net/nimble/host had grown > quite large over the last few months. During development of the host, > most focus was put on correctness and maintainability, while speed and > size considerations were placed in the background. Now that the host is > reasonably complete, it is a good time to start optimizing, with the > hope that the unit tests will protect against any regressions. > > The commit consists of two major changes: > > 1. Use packed structs for over-the-air ATT and L2CAP commands. > 2. Compile-out most assertions unless BLE_HS_DEBUG is defined. > > Regarding 1: > > Prior to this change, the host code was manually > marshalling fields between buffers and properly-aligned structs. This > had the benefit of being defined behavior according to the C standard, > but at the cost of increased code size. By using the gcc __packed__ > attribute, the compiler is able to take advantage of the Cortex-M's > ability to perform unaligned accesses, resulting in a substantial code > savings. > > Regarding 2: > > The host code makes liberal use of the assert() macro. > Each invocation of the assert() macro was adding a surprising amount of > code to the resulting binary. The size increase is not caused by > strings being embedded in the binary; the baselibc assert() does not include > the expression text in its expansion, and gcc collapses duplicate > filename strings. The size increase is just caused by the comparison > and branch implicit in the assert macro. > > The asserts in the host code are plentiful and conservative. They > caused an unwelcome increase in code size, but I didn't want to remove > them entirely because they are valuable during regression testing. The > standard assert() macro compiles to nothing if the NDEBUG symbol is > defined, but I wanted more control over which asserts actually get > disabled. As a compromise, I added a second type of assertion that only > gets compiled in if the BLE_HS_DEBUG symbol is defined. > > I think the concept of multiple assert levels will be useful to other > packages and that it should be added to the core OS. For this reason, I > intentionally made the new assert macro names ugly, as I hope to replace > them with something that isn't host-specific. > > Here are the two macros I added: > > #if BLE_HS_DEBUG > #define BLE_HS_DBG_ASSERT(x) assert(x) > #define BLE_HS_DBG_ASSERT_EVAL(x) assert(x) > #else > #define BLE_HS_DBG_ASSERT(x) > #define BLE_HS_DBG_ASSERT_EVAL(x) ((void)(x)) > #endif > > > BLE_HS_DBG_ASSERT(x) is self-explanatory; it asserts if the necessary > debug symbol is defined. > > BLE_HS_DBG_ASSERT_EVAL(x) is less obvious. If the BLE_HS_DEBUG symbol > is defined, this macro has the same effect as the first one. If the > symbol is not defined, this macro evaluates its argument and throws away > the result. The reason this macro exists is to prevent gcc from raising > "variable set but not used" warnings in code like the following: > > rc = operation_that_should_never_fail(); > BLE_HS_DBG_ASSERT_EVAL(rc == SUCCESS); > > Note that the BLE_HS_DBG_ASSERT_EVAL() macro differs from the standard > assert() macro in this respect. The standard assert() macro does not > evalute its argument if NDEBUG is defined, which allows you to write > code like the following: > > assert(expensive_sanity_check() == SUCCESS); > > Future changes: > > There are a number of future changes planned for further reducing the > host code size. > > * Pack the HCI command and event structs. > * Rework (or remove / replace) the generic FSM implementation; the > code has outgrown this facility. > > If you have made this far and have any suggestions of your own, they > would be much appreciated, so please feel free to share them. > > Thanks, > Chris
