Allow running mbed_highprio_event_queue() in main thread

[janjongboom]

Description

I can disable the normal event queue thread by setting MBED_CONF_EVENTS_SHARED_DISPATCH_FROM_APPLICATION=1, but the high prio queue is always running from a separate thread. But what if my application does not require this, or is very low on memory? I want to be able to disable it. I think a good way would be to introduce MBED_CONF_EVENTS_ENABLE_HIGHPRIO_QUEUE and if set to 0 it returns mbed_event_queue().

E.g.:

diff --git a/events/mbed_lib.json b/events/mbed_lib.json
index cf9f659403..7b644a820d 100644
--- a/events/mbed_lib.json
+++ b/events/mbed_lib.json
@@ -25,6 +25,10 @@
         "use-lowpower-timer-ticker": {
             "help": "Enable use of low power timer and ticker classes in non-RTOS builds. May reduce the accuracy of the event queue. In RTOS builds, the RTOS tick count is used, and this configuration option has no effect.",
             "value": 0
+        },
+        "enable-highprio-queue": {
+            "help": "Enables the high priority queue. Disabling this feature will return the mbed_event_queue() when requesting the high prio queue.",
+            "value": 1
         }
     }
 }
diff --git a/events/mbed_shared_queues.cpp b/events/mbed_shared_queues.cpp
index 1fc6a079b9..589a476158 100644
--- a/events/mbed_shared_queues.cpp
+++ b/events/mbed_shared_queues.cpp
@@ -69,7 +69,11 @@ EventQueue *mbed_event_queue()
 #ifdef MBED_CONF_RTOS_PRESENT
 EventQueue *mbed_highprio_event_queue()
 {
-    return do_shared_event_queue_with_thread<osPriorityHigh, MBED_CONF_EVENTS_SHARED_HIGHPRIO_EVENTSIZE, MBED_CONF_EVENTS_SHARED_HIGHPRIO_STACKSIZE>("shared_highprio_event_queue");
+#if MBED_CONF_EVENTS_ENABLE_HIGHPRIO_QUEUE == 1
+    return do_shared_event_queue_with_thread<osPriorityHigh, MBED_CONF_EVENTS_SHARED_HIGHPRIO_EVENTSIZE, MBED_CONF_EVENTS_SHARED_HIGHPRIO_STACKSIZE>("shared_highprio_event_queue");
+#else
+    return mbed_event_queue();
+#endif
 }
 #endif

Issue request type

[ ] Question
[X] Enhancement
[ ] Bug

[kjbracey]

If it needs to be high priority, and you're using the normal event queue for things with standard timing characteristics, then combining them won't work. The high-priority user won't be getting the latency requirement they apparently need - they'll be stuck behind all the people taking tens of milliseconds per event.

If it does work, then it would seem that particular user doesn't need to be high priority at all, and should be changed to use the normal. Or it should have a flag to configure it to use normal optionally with documentation on whatever performance provisos there are. Just doing it globally feels dangerous, but maybe in an emergency?

Now, there is an alternative in the opposite direction that can work - telling a driver to use interrupts directly instead of the high priority queue. The Nanostack/Pelion client HAL has that option, called (obscurely) nanostack-hal.critical-section-usable-from-interrupt. The trade-off there is that it increases interrupt latency, possibly to the extent of losing serial. In practice it works for non-mesh client lite builds, where there's no 802.15.4 radio SPI traffic running in those critical sections.

The high priority queue and normal priority queue pair are an analogue to the two "interrupt" and "foreground" contexts you'd get in a bare-metal build, but pushing the high-priority down to thread to be a bit nicer to latency - avoiding that serial loss when you a high-priority SPI transfer. So you always have the option to push it back up to interrupt, but it's a tad marginal. Unfortunately we don't have a way of transparently making "high priority" be thread or interrupt at the in the main code, although @c1728p9 has got something along those lines in USB, I believe.

[ciarmcom]

Internal Jira reference: https://jira.arm.com/browse/MBOCUSTRIA-1091

[kjbracey]

In your application where you're experimenting with this option - who's using the high-priority queue and who's using the normal one?

[janjongboom]

@kjbracey-arm I'm not sure. I'm using Pelion Client with Mbed OS 5.12 on DISCO-L475VG-IOT01A1 board and nothing seems to break when disabling highprio queue.

[kjbracey]

Low memory client configurations are @TeroJaasko's speciality. Maybe he has input.

The high priority queue is probably just being initialised for the Nanostack HAL's "platform timer". I guess if that timer is only being used to schedule "normal priority" work in this system, and it's not being used for any driver-type purposes, it can be shoved down to the normal queue.

I think our assumption was though that in a system the high priority event queue would be in use by drivers, so would not cost extra for that purpose - it's only the transition from "no users" to "1 user" which costs you something. But a lot of drivers are still creating their own high-priority threads rather than sharing the high-priority one. A better line of attack might be to find any such driver in your system, and make it share high priority with Nanostack's platform timer.

You may also want to turn on nanostack-hal.event-loop-use-mbed-events, if it isn't already, which does push Nanostack event dispatch into the normal event queue, rather than its own thread, potentially saving another thread. That doesn't affect the timing though - the timing still comes from the "platform timer" rather than the Mbed OS event queue's timing.

[janjongboom]

@kjbracey-arm Yes, already doing that. An overview of changes I made is in: https://github.com/janjongboom/pelion-ready-plus-azure.

[kjbracey]

Another line of attack here would be to improve event-loop-use-mbed-events to be more fully integrated, as it was for Mbed OS 3 with MINAR.

At the minute it's kind of a minimal integration because it doesn't let the Mbed OS event loop also take over scheduling - it's just done minimally in the "bare-metal-like" Nanostack porting layer, rather than having a complete higher-level reimplementation of the API like for MINAR.

If we did that, then the Nanostack HAL platform timer wouldn't be needed. And there would be other power/latency benefits from hooking into the standard system. Running your own Timers like that is not ideal.

[TeroJaasko]

@kjbracey-arm I agree wholeheartedly. IMHO too it is the user of those queues which should decide if and how they can tolerate the latencies if there are no separate message queues.

The mbed_highprio_event_queue() call likely comes from arm_hal_timer.cpp, arm_hal_fhss_timer.cpp or on a Pelion application, the pal_plat_network.cpp. The last one is using the high priority queue by default just because the high-priority thread already always exists and the code did not want to waste memory. All these call sites have alternative paths for "nanostack-hal.critical-section-usable-from-interrupt": true case.

If the system works with the attached hack, it should work also with "nanostack-hal.critical-section-usable-from-interrupt: true", which effectively also remove calls to mbed_highprio_event_queue(), some parts of RTOS and as a bonus, makes code a bit closer to compile without MBED_CONF_RTOS_PRESENT -define.

[janjongboom]

@TeroJaasko @kjbracey-arm But right now I don't get this choice. The high prio queue is always started up. What if none of my libraries / drivers use the queue? The thread is still started and still steals a bunch of RAM.

[kjbracey]

The high prio queue is always started up. What if none of my libraries / drivers use the queue? The thread is still started and still steals a bunch of RAM.

It's only started if it's used. It is being used by the Nanostack event loop, which is being used by Pelion Client.

First call to mbed_event_queue() or mbed_highprio_event_queue() starts that queue on a new thread.

(Exception being mbed_event_queue() won't start a thread if events.shared-dispatch-from-application is true).

[janjongboom]

@kjbracey-arm Ah, ok. Thanks for specifying.

[linlingao]

@janjongboom I'm not clear if further enhancement is till needed. Please reopen if so.