Reduce CPU usage of data display. #34
Conversation
My sensor is providing many data on 100Hz, and I see significant CPU usage compared to old CuteCom. I think the problem is in QPlainTextEdit. This PR is only a suggestion. It does batch insertText() operations on timer. CPU usage is reduced from 40% to 10%.
Now incomplete line is being removed correctly before displaying block of hex data.
|
I see that easier solution could be just: But with massive output in hex mode master version (and this easier solution also doesn't help) make CueCom quickly hang when I click output text with mouse. But foregoing commits are free of this hanging. |
There was a problem hiding this comment.
I reviewed this PR and I wrote my thoughts in the first commit. I explained why is better to use the single shot timer instead of the repetitive one, so if we agree then the last commit (b7c5e73) should be reverted.
I'm not quite sure for what the second commit (4c56d91) is for. It seems that it fixes something that I'm not aware of, so can you write some detailed information? In my opinion, if it is a bug please open new issue.
| @@ -59,6 +59,9 @@ DataDisplay::DataDisplay(QWidget *parent) | |||
|
|
|||
| connect(m_searchPanel, &SearchPanel::findNext, this, &DataDisplay::find); | |||
| connect(m_searchPanel, &SearchPanel::textEntered, m_highlighter, &DataHighlighter::setSearchString); | |||
|
|
|||
| connect(&m_timer, SIGNAL(timeout()), this, SLOT(BlockReady())); | |||
There was a problem hiding this comment.
Please use new syntax to connect signals with slots.
| @@ -59,6 +59,9 @@ DataDisplay::DataDisplay(QWidget *parent) | |||
|
|
|||
| connect(m_searchPanel, &SearchPanel::findNext, this, &DataDisplay::find); | |||
| connect(m_searchPanel, &SearchPanel::textEntered, m_highlighter, &DataHighlighter::setSearchString); | |||
|
|
|||
| connect(&m_timer, SIGNAL(timeout()), this, SLOT(BlockReady())); | |||
| m_timer.start(100); | |||
There was a problem hiding this comment.
Using QTimer, in my opinion, is the best way to buffer incoming characters before printing them out. But this use case of QTimer Causes repetitive call a timeout slot even when serial device is closed. So, please change to a single shot configuration, set interval value (I think 100ms is the maximum, but I would prefer to set smaller interval, maybe value between 50ms and 75ms) and finally call the start() from the displayData() only when the timer is inactive.
There was a problem hiding this comment.
Thank you for the comments, I will add your corrections to my branch. As I'm new to GitHub, please answer, should I do corrections as new commit, or may I make a fixup using git rebase -i and push?
There was a problem hiding this comment.
When you do rebase that means you are rewriting the history. If you will push that onto GitHub then all old commits and theirs inline comments will disappear, so please make an another commit.
| @@ -129,6 +130,10 @@ class DataDisplay : public QWidget | |||
|
|
|||
| QVector<QTime> *m_timestamps; | |||
| DataHighlighter *m_highlighter; | |||
| QTimer m_timer; | |||
There was a problem hiding this comment.
Please change to more descriptive name, e.g. m_BufferingIncomingDataTimer, and provide short comment which will describe what this field does.
| QTimer m_timer; | ||
|
|
||
| private slots: | ||
| void BlockReady(void); |
There was a problem hiding this comment.
Please change to more descriptive name and provide a short comment.
- switched to singleshot timer 70ms; - renames; - call displayDataFromBuffer() also on changing display mode, for nice output when hex switches to ascii and back;
Hi.
My sensor is providing many data on 100Hz,
and I see significant CPU usage compared to old CuteCom.
I think the problem is in QPlainTextEdit.
This PR is only a suggestion.
It does batch insertText() operations on timer.
CPU usage is reduced from 40% to 10%.
Dmitry