Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Spectrum analyzer update #5160
Spectrum analyzer update #5160
Changes from 16 commits
9be31e0
4e2f168
f5eda87
7d630e1
249d161
25aa537
3707eeb
cb3e701
8d639e5
36feb65
b650838
42d74db
9dd9ef0
6a5089d
6939702
11013cc
edefc93
e3c89d5
5b1f28c
7cf189c
11bb2c7
2963fb6
6dd2619
6856b3d
0d56ae8
228dd2f
22e9163
dc2bd91
8739abb
7a0fc5a
bf793da
ad49d36
a0acc8a
8ca05a3
c694277
0caa748
a7388b1
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is likely (dependent on the implementation) not realtime safe. If you want to keep the realtime constraint up, it may be better to leave the other thread busy waiting 😕 I'm still thinking about a good solution, it's not easy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain how exactly does it cause a behavior that is not real-time safe?
Assuming you the effect processing thread has real-time priority (otherwise it does not matter), even if the
QWaitCondition
uses a mutex internally and it causes a syscall which returns control to the OS kernel, the kernel will just handle the syscall and return control back to your process (because you have real-time priority).The way the mutex is used guarantees that it will always be free and available, because it only prevents multiple threads from trying to call the "wake up" at the same time. In this case there is only one thread such thread, so there is nothing else that could take the mutex, i.e. it will be always acquired instantly.
We also discussed delays that could occur because of context switch from user space to kernel space -- but these will occur anyway, since to achieve multi-tasking the kernel has to re-schedule what processes and threads are running every few milliseconds.
So, to summarize: syscall: no delay because you have realtime priority and context switches occur all the time anyway. Mutex: no delay because it is always free to take. So I still don't see the problem.
On the other hand, busy waiting has definite drawbacks: with high poll rate it needlessly burns CPU time (with cumulative effect over multiple plugin instances), and when the poll rate is low it wastes available CPU time by sleeping too long when you could have been processing data already.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm really sorry, I've changed this text, but it seemed like my internet connection was broken at the time of editing.
I agree to all what you say, if you replace mutex by condition variable. Especially, I think you're right that this code does not hurt any of the 3 arguments not to use "mutexes" mentioned in the article from Ross Bencina. So, in theory, we could whitelist waking up threads in condition variables in stoat.
In practice, however:
QWaitCondition
uses apthread_condition
variable and apthread_mutex_t
.pthread_cond_wait
uses__pthread_cond_mutex_lock
(e.g. a mutex lock) plus a spinlock.I imagine both cases are equally dangerous as the ones with the
sem_post
? @fundamental should we whitelistpthread_cond_wait
, or do you consider it worse thansem_post
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The programming patterns that pthread_cond_wait tend to have worse semantics than sem_post. sem_post does involve an internal futex lock, but that lock is virtually never blocked while pthread_cond_wait assumes that the section should fairly regularly block waiting for an event to occur.