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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
