zephyriot / zep-jira14

0 stars 0 forks source link

Unnecessary code footprint bloat. #1640

Open nashif opened 7 years ago

nashif commented 7 years ago

Reported by Marcus Shawcroft:

The code base makes extensive use of the 'static inline' idiom. Adding the -Winline option to a build instructs the compiler to issue a diagnostic each time an invocation of a function marked inline was not actually inlined. Building one of the sample applications typically generates order of around one hundred such diagnostics (I experimented with samples/net/dhcpv4).

A summary of the distribution of these inline functions where the compiler chooses not to inline looks like this (for samples/net/dhcpv4 on frdm_k64f):

147 net_buf_user_data
 23 net_nbuf_icmp_data
 22 _ready_thread
 22 pinmux_pin_set
 15 _abort_timeout
 13 net_nbuf_ll
 11 net_nbuf_iface
 10 net_nbuf_udp_data

...

If the compiler does not inline all calls to a specific function within one translation unit (and in a few other corner cases) the compiler will emit an outline copy of the function. The zephyr coding idiom typically uses 'static inline' rather than the 'c99 extern inline' idom. This means that each translation unit that references an inline function where the compiler does not inline the function will gain a static copy of the function.

When the final image is linked each translation unit retains its private copy of these functions. For example:

$ arm-none-eabi-objdump -d ./outdir/frdm_k64f/zephyr.elf | grep net_buf_user_data 00002804 : 00002810 : 0000281c : 00002828 : 00002834 : 00002840 : 0000284c : 00002858 : 28da: f7ff ff93 bl 2804

ie, in this case we have 8 copies of net_buf_user_data linked into the image.

This unnecessary replication of function implementations looks like a reasonably small percentage of the overall image size, but, it is literally 'a waste of space'

(Imported from Jira ZEP-1790)

nashif commented 7 years ago

by Marcus Shawcroft:

I looked briefly at why we use 'static inline' rather than the c99 'extern inline' type model. This model breaks due to multiple definitions of the outline functions, I havn't dug into this properly, but it appears that the multiple definition issue arizes as a result of our extensive use of ld -r in the build process.

nashif commented 7 years ago

by Jukka Rissanen:

There was lot of uses of net_buf_user_data() and we should definitely have it inlined. If you look at that function it it is just doing a cast which is in turn used by nbuf.h to do more casts.

nashif commented 7 years ago

by Marcus Shawcroft:

net_buf_user_data() is not the issue here, there are plenty of other examples of functions where this issue occurs. I briefly experimented with flipping from inline static to extern static to see if we could get to a single definition, but this results in multiple definitions (which I did not expect).

I think this is a build system issue rather than a 'networking' issue.