-
Notifications
You must be signed in to change notification settings - Fork 814
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support notify hook for thread resume and suspend #426
Comments
Hi @greglink. Thank you for opening this issue. I appreciate your offer to implement this. Let's see what the project team thinks of your idea. @eclipse-threadx/iot-threadx-committers Please review the proposal and provide your feedback. |
Most developers would not find this helpful, so anything added must be under conditional compilation. This is also a critical real-time code path, so we must be extra cautious. That said, perhaps extension macros (defaulted to white space but able to be overridden by the developer) might be safer than function pointers and result in less overhead. You could define these as function calls or directly toggle pins from inside these routines. Interested to hear your thoughts.... |
I agree with your assessment - I had planned to include it behind conditional compilation to match the style of other such hooks in the code, but I'm also open to extension macros. The only downside to extension macros (which I agree would be faster - no need to check for a non-NULL function pointer, nor need to even store it in the thread block) is that it doesn't seem to match the way other similar notify hooks are handled. The entry_exit hook is evaluated (and optionally called) every time a thread enters and leaves the CPU, which can occur multiple times if said thread is pre-empted repeatedly. My belief is that a suspend/resume hook would likely be called much less frequently, and thus, while it would be in the critical execution path, it should also be lower impact than the entry_exit hook already in the code, and therefore, not 'overly much'. |
I agree Greg that the entry/exit function is more overhead-wise since it happens at every context switch. However, the historical reason for the function pointer in this case was because not all developers had access to the source code for rebuilding/overriding. That is moot now given the source is in the public domain. Hence, my recommendation for a couple of extension macros.... |
That makes good sense. It'll take me a bit to get a first draft PR up, then (I actually have to do the work), but I'm happy to use that technique, and expect and appreciate any feedback on improving it, as I won't "just" be mimicking the existing callback techniques. |
I suggest a couple macros in tx_api.h defined to whitespace: #ifndef TX_THREAD_SYSTEM_RESUME_EXTENSION #ifndef TX_THREAD_SYSTEM_SUSPEND_EXTENSION And then simply drop in TX_THREAD_SYSTEM_RESUME_EXTENSION and TX_THREAD_SYSTEM_SUSPEND_EXTENSION in the actual C code where you want to add the instrumentation. For resumption, I recommend sometime after the thread is completely on the ready list and for suspension sometime after the thread is completely removed from the ready list. I hope that helps! |
Problem Description
I'd like to monitor/observe (at runtime) the latency from a thread becoming ready for any reason (timer, semaphore, etc) until it begins execution, and then voluntarily suspends itself. These metrics are useful for ensuring a responsive real time operating system, and can be emitted as telemetry in formats like a histogram or high water mark to indicate potential delays or timing issues.
Describe the solution you'd like
I'd like to add/install optional callback/notify hooks in a similar fashion to the tx_thread_entry_exit_notify_ hooks, only this time for suspend and resume.
Describe alternatives you've considered
I could theoretically instrument all potential resume initiators (e.g. queues, semaphores, ...) but it seems like instrumenting at tx_thread_resume or tx_thread_system_resume might be the best/simplest location. I also considered emitting trace information, and scanning the trace information at runtime, but this seemed very heavy, as I couldn't restrict trace emission to only those events.
Additional context
I'm volunteering to write the PR, if someone can re-assure me that this won't be a rejected request for performance reasons, and inform me of whether _tx_thread_resume, _tx_thread_system_resume, or some other location is the best place to install the hook.
The text was updated successfully, but these errors were encountered: