Skip to content
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

Real-time data performance mods #205

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

kubark42
Copy link
Contributor

@kubark42 kubark42 commented Jan 20, 2022

VESC-Tool would consume >70% processor while idle, but receiving real-time data.

These changes address the vast bulk of the idle processor load. I see only around 12% idle, for a reduction of over 80%.

Note, however, that while viewing the real-time graph processor load is still extremely high.

mainwindow.cpp Outdated
ui->dispCurrent->setVal(values.current_motor);
ui->dispDuty->setVal(values.duty_now * 100.0);
mMcValues = values;
shouldSetDisplayBars = true;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename this to shouldUpdateDisplayBars

@kubark42 kubark42 force-pushed the vesc_realtime_performance branch 2 times, most recently from 13302b3 to 475cb20 Compare January 29, 2022 16:47
Too high of a frequency can't be easily read and burns up lots of CPU in
redraw events.
@Teslafly
Copy link
Contributor

Teslafly commented Nov 6, 2022

is this still relevant? or ready to merge?

@kubark42
Copy link
Contributor Author

kubark42 commented Nov 6, 2022

is this still relevant? or ready to merge?

Since it's Qt 5.15 behavior on macOS, it is still relevant to macOS. It might be a resolved problem on Qt 6.x, but
last I heard from @vedderb's he intends to keep with Qt 5.15 in response to the Qt Corp's nastiness regarding licensing with Qt 6..

@vedderb
Copy link
Owner

vedderb commented Nov 7, 2022

I think we talked about this on the discord back then. I also added some improvements since that greatly increased the performance on slow computers. This does a bit more, but the drawback is that when you leave a rt data page while sampling and go back to it the data that arrived in that time will be missing, which is why I didn't want to include this pr.

I just tried it now, and when I sample RT, IMU, APP and BMS data at the same time and have the plot open on fullscreen VESC Tool draws about 1% of my total CPU (25-40% on one of the cores at worst). When the plot page is not open and sampling is active that core goes down to almost nothing.

@kubark42
Copy link
Contributor Author

kubark42 commented Nov 7, 2022

We did, here's the link to that convo: https://discord.com/channels/904830990319485030/910253910655123566/937017928417685506

Quoting from that link:

changes ... pushed to master were a nice... upgrade... but even with those in place my CPU idle load hardly dropped (72%-->69%)... Whereas with this PR I see the processor load drop to 12-15%.

IIRC, you use Linux and thus the Qt subsystem idles substantially better for you than for macOS users. I don't think we ever discovered why Qt 5 behaves that way, but I know this problematic Qt CPU usage affects many more Qt 5 applications than just VESC-Tool.

I also don't remember us finding a better solution than this PR to address the CPU issues on macOS, so to answer @Teslafly's question the PR is still relevant to a subset of VESC-Tool users. That doesn't preclude closing it, since if it's never going to get merged it doesn't make sense to leave it lingering open. The problem and work-around are documented here, so macOS users who roll their own VESC-Tool can use this code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants