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


Reply via email to