xiaoxiang781216 commented on a change in pull request #5154: URL: https://github.com/apache/incubator-nuttx/pull/5154#discussion_r778164713
########## File path: include/nuttx/net/usrsock.h ########## @@ -227,9 +228,9 @@ begin_packed_struct struct usrsock_message_req_ack_s { struct usrsock_message_common_s head; - uint8_t xid; - uint8_t reserved; - int32_t result; + int16_t reserved; Review comment: > In case if you use packed structures compiler does not take CPU alignment into account that generate "special" code to access unaligned data Most modem CPU(e.g. ARMv7 A/R/M and RISCV) support the unalignment access at the hardware level, but the unaligned access still need the additional bus transaction than the aligned access. Manual alignment here could improve the performance. > Such so called "optimization" costed me a lot when I was building communication in multi-core system between 32-bit and 64-bit cores :) and I would not recommend to apply it in any types generic communication. > > I see that no "usrsock_swapX" interface is not available, so the interface currently supports messaging between same endian systems (that is friendly speaking not always true for some multi-core systems). > Multi-core system is more fixed than the traditional network environment. The IC designer normally enforce all CPU/MCU/DSP share the same endianness in one SOC. Since multi-core has much better the speed/latency than multi-machine, it is important to avoid the unnecessary swapX in this case. > I will let other reviewers to comment on this. > > What I'm curious why in some cases the elements are sorted in type size increase and in other with type size decrease? Like here and > > ``` > begin_packed_struct struct usrsock_request_common_s > { > uint64_t xid; > int8_t reqid; > int8_t reserved; > } end_packed_struct; > ``` > > And > > ``` > begin_packed_struct struct usrsock_request_sendto_s > { > struct usrsock_request_common_s head; > int16_t usockid; > int32_t flags; > uint32_t buflen; > uint16_t addrlen; > } end_packed_struct; > begin_packed_struct struct usrsock_request_recvfrom_s > { > struct usrsock_request_common_s head; > int16_t usockid; > int32_t flags; > uint32_t max_buflen; > uint16_t max_addrlen; > } end_packed_struct; > ``` > > even do not care about sorting. Since the message/request split into two structure, ordering the field by reversed size in each structure can't get the optimized result. For example: 1. usrsock_request_common_s has 10Byte size 2. First field of usrsock_request_sendto_s/usrsock_request_recvfrom_s is int16_t which is aligned correctly here but int32_t isn't. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: commits-unsubscr...@nuttx.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org