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

High CPU usage in release mode #2

Open
Jon-Salmon opened this issue Nov 8, 2024 · 10 comments
Open

High CPU usage in release mode #2

Jon-Salmon opened this issue Nov 8, 2024 · 10 comments

Comments

@Jon-Salmon
Copy link

When an app that uses this plugin is running in release mode, the CPU usage is very high. My app usually runs with <1% CPU usage when idle. Adding this plugin raises it to ~10% CPU usage when idle and in the background. Oddly, this is not so noticable in other modes (including profile).

I suspect the issue is with how the isolate is used in device_monitor.dart, and I suspect that most of the logic in there could be replaced by using a plugin like this https://pub.dev/packages/device_manager (or using a similar implementation). Flutter exposes some Windows window notifications to the windows plugin system.

@chive-gherkin
Copy link

I am experiencing high CPU usage with this plugin in release, debug and profile modes
The high CPU usage is constant and consistent for the lifetime of my Flutter app
I have removed the plugin dependency of [flutter_midi_command: ^0.5.1] from [pubspec.yaml] and all my associated code to prove that this plugin is the root cause

This issue only occurs for the Windows build of my application. The Linux build runs at idle CPU as and when expected

Using Task Manager > Details > (Right click) My App Process > Set affinity to 1 CPU then that CPU runs at 100% continuously

Given the Linux version of the plugin does not support Bluetooth I wonder if that could be relevant. I have found a ticket requesting Bluetooth support to be optional when initialising the plugin. Is that straight forward to implement?

Please note my development machine (AMD Ryzen 5) does not have any onboard support for Bluetooth

Thank you very much for developing and making this plugin available! It does exactly what I want and was very straight forward to work with

I am very keen to help resolve this issue where possible, so please let me know if there is anything I can do

@chive-gherkin
Copy link

chive-gherkin commented Nov 27, 2024

Upon further review and now with a better understanding, I can concur with Jon's comment above

Commenting out line 27 from [%localappdata%\Pub\Cache\hosted\pub.dev\flutter_midi_command_windows-0.1.1\lib\device_monitor.dart] avoids the high CPU issue

In my cached copy line 27 now reads:
// Isolate.spawn(_device_monitor, _receiver.sendPort);

Placing a breakpoint in [device_monitor.dart] allowed me to step into the _device_monitor() function and get the VS Code debugger stuck in the infinite while (run) {} loop (line 66..84)

As an aside, on every debugging session I had an unknown issue getting the debugger to enter the main() entry point of my app. I had to click the 'All Exceptions' breakpoint option to move the debugger on. This can now be explained as the debugger was locked in the isolate _device_monitor() thread. The action above brought the debugger focus to the main() entry point and allowed me to proceed.

@mortenboye
Copy link
Contributor

Thank you @Jon-Salmon and @chive-gherkin, for reporting this.

I like Jon's suggestion of switching to an existing plugin for this functionality. If either of you are interested in pursuing that solution, that would be much welcome.
I am currently quite pressed on time and can't say when I might get around to fix this.

@jforsten
Copy link

jforsten commented Jan 5, 2025

Noticed the same. Temporary fix is to add Sleep(1) to that while(run) loop in the DeviceMonitor class. Other solution was to change the non-blocking PeekMessage to GetMessage:

//bRet = PeekMessage(msg, NULL, 0, 0, PEEK_MESSAGE_REMOVE_TYPE.PM_REMOVE);
bRet = GetMessage(msg, NULL, 0, 0);

Both of these will bring the CPU usage close to 0% from 15%-20%. And based on quick tests everything works as expected.
I'm not expert in windows prorgramming but can refer to this: https://stackoverflow.com/questions/33948837/win32-application-with-high-cpu-usage

As this code is apparently used to detect attach/detach of MIDI devices, it is clear that using busy loop for that is not really needed, so would think that using the blocking GetMessage in its own thread (or isolate) is totally fine.

@jforsten
Copy link

jforsten commented Jan 5, 2025

After more testing it looks that the GetMessage blocks the thread and will actually leave the isolate running when closing the app window, so it requires little more work. Quick fix is then to add the Sleep(10) (or maybe even more?) in the loop.

@Jon-Salmon
Copy link
Author

I had a bit of a dig into this a little while ago and came to the conclusion that the windows implementation is only using the extra thread in order to detect device connections. This information is exposed by the Flutter engine anyway so it should be easy enough hook into (with something like this https://pub.dev/packages/device_manager) and therefore vastly simplify the logic in the Windows implementation here. I'm afraid I'm unlikely to have time to look into actually implementing this myself for a few months.

@jforsten
Copy link

jforsten commented Jan 5, 2025

That seems a much better solution. I'll see if I can find any time to contribute to the fix.

@mortenboye
Copy link
Contributor

Made a very quick PR. Haven't tested it yet, but feel free to try out and contribute if possible.

@jforsten
Copy link

jforsten commented Jan 6, 2025

I played around with that PR and first noticed that device_manager package 0.0.2 does not compile (bojidartonchev/device_manager#2) as it used 'hashValues()' instead 'Object.hash()'. After fixing that I managed to run the device_manager example which seems to work and there was no CPU issue with it which was good sign.

I applied the PR changes to the 'flutter_midi_command_windows' package and managed to compile the 'flutter_midi_command' example project with the fixed device_manager package, but unfortunately there was some exceptions ( Unhandled Exception: MissingPluginException(No implementation found for method isNetworkSessionEnabled on channel plugins.invisiblewrench.com/flutter_midi_command) and the example application was just hanging with progress indicator.

As not sure if I was able to setup the changes properly (I'm new to flutter and it is not trivial to work with multiple package dependencies while making changes some of them) I'm more than happy to take any tips how to make any progress on this.

@mortenboye
Copy link
Contributor

I have updated the PR to base on bojidartonchev/device_manager#3 instead of main of device_manager.
This fixes the compilation issue.
There is still a timing/initialization issue i haven't had time to dig into, but solved it temporarily by adding a small delay before starting device_manager 🙈

Feel free to give it a try.

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

No branches or pull requests

4 participants