Fix mbed-client behaviour in irq contexts #2530
Conversation
During debugging, the tr_debug call in M2MConnectionHandlerPimpl::socket_event took an unacceptable amount of time to complete, causing bytes traveling through the esp8266's serial connection to be dropped. This was unnoticable in lwip's network-socket implementation due to not occuring in an interrupt context.
Replaced malloc calls in interrupt contexts with allocations from a local memory pool using rtos::MemoryPool
| @@ -310,9 +312,7 @@ int8_t M2MConnectionHandlerPimpl::connection_tasklet_handler() | |||
|
|
|||
| void M2MConnectionHandlerPimpl::socket_event() | |||
There was a problem hiding this comment.
is this called from irq context? then it make sense to minimize code here, like remove trace and allocation related calls,,, but how faster this memory_pool really are? any numbers?
There was a problem hiding this comment.
I haven't measured their performance, it would be interesting to see the numbers.
Although the issue isn't with average performance, but with jitter. As a simple linked-list, the memory pool garuntees constant runtime, where malloc may have an outlier (in the case of coalescing lazily for example) but be faster on average. This outlier may disrupt interrupt based communication.
|
I think this should be done only for mbed-client-classic.. |
|
Sorry, I don't quite understand. Is this pr fine as is? |
|
@geky Any fix PR on mbed-client-classic or mbed-client should be done on those repos directly. |
|
@yogpan01, ok thanks for the clarification. I'll move this over to mbed-client-classic. Are there bulk prs to bring mbed-os up to date? |
|
@geky Closing this as I understood correctly all the messages above. If not, please reopen |
|
Moved to ARMmbed/mbed-client-classic#31 |
Should resolve ARMmbed/mbed-os-example-client#75
mbed-client defines
M2MConnectionHandlerPimpl::socket_eventfor handling the event passed through theUDPSocket::attachfunction. Unfortunately, the previous implementation contained both atr_debugcall andmalloccall that were unacceptably long-running and caused UART bytes to be dropped.This issue was not unnoticable in lwip's implementation. We could update lwip to dispatch events in a critical section, insuring correct behaviour, although this would introduce unnecessary jitter in existing code.
cc @yogpan01, Should I be making this pr to https://github.com/armmbed/mbed-client-classic as well? Let me know how to best submit patches for this code.