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

get available CPUs and RAM from daemon when launching a new instance in GUI #3867

Merged
merged 8 commits into from
Jan 16, 2025

Conversation

levkropp
Copy link
Contributor

@levkropp levkropp commented Jan 6, 2025

This PR addresses #3838 by improving how the GUI gets system resource information. Instead of the GUI directly checking system specs (like CPU count and memory size), we now pull that data from the daemon via a new gRPC call. This keeps the logic centralized in the daemon, which already handles disk space reporting, so it makes sense to extend that to cover CPUs and memory as well.

Daemon updates:

  • Now uses POSIX system calls, calling _SC_NPROCESSORS_ONLN and _SC_PHYS_PAGES * _SC_PAGESIZE to get CPU count and total ram size respectively. This info is now included in the DaemonInfoReply message.
  • These system calls are only implemented for Linux and MacOS. We'll need separate implementations for Windows.

GUI changes:

  • Updated the LaunchForm to show available CPUs, memory, and storage in the UI by using daemonInfoProvider.
  • Update disk, cpu, and memory sliders to be built with values from daemonInfoProvider

@levkropp levkropp force-pushed the daemon-cpu-disk-usage branch from 5867f84 to af771ec Compare January 6, 2025 19:34
@andrei-toterman andrei-toterman self-requested a review January 6, 2025 20:02
Copy link

codecov bot commented Jan 6, 2025

Codecov Report

Attention: Patch coverage is 66.66667% with 2 lines in your changes missing coverage. Please review.

Project coverage is 89.02%. Comparing base (eb18d70) to head (96f5187).
Report is 11 commits behind head on main.

Files with missing lines Patch % Lines
src/daemon/daemon.cpp 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3867      +/-   ##
==========================================
- Coverage   89.03%   89.02%   -0.01%     
==========================================
  Files         255      255              
  Lines       14577    14583       +6     
==========================================
+ Hits        12978    12982       +4     
- Misses       1599     1601       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@levkropp levkropp force-pushed the daemon-cpu-disk-usage branch 6 times, most recently from bad74f2 to d29c1fe Compare January 6, 2025 22:59
src/rpc/multipass.proto Outdated Show resolved Hide resolved
src/daemon/daemon.h Outdated Show resolved Hide resolved
src/daemon/daemon.cpp Outdated Show resolved Hide resolved
src/client/gui/lib/catalogue/launch_form.dart Outdated Show resolved Hide resolved
src/client/gui/lib/vm_details/cpus_slider.dart Outdated Show resolved Hide resolved
src/client/gui/lib/vm_details/ram_slider.dart Outdated Show resolved Hide resolved
@levkropp levkropp force-pushed the daemon-cpu-disk-usage branch from 13c9059 to 9874339 Compare January 7, 2025 21:40
@levkropp levkropp marked this pull request as ready for review January 8, 2025 13:29
src/client/gui/lib/providers.dart Outdated Show resolved Hide resolved
src/daemon/daemon.h Outdated Show resolved Hide resolved
src/platform/platform_linux.cpp Outdated Show resolved Hide resolved
src/platform/platform_linux.cpp Outdated Show resolved Hide resolved
src/client/gui/lib/vm_details/cpus_slider.dart Outdated Show resolved Hide resolved
src/client/gui/lib/vm_details/disk_slider.dart Outdated Show resolved Hide resolved
@levkropp levkropp force-pushed the daemon-cpu-disk-usage branch 5 times, most recently from 78439bd to 94d3af8 Compare January 9, 2025 18:06
tests/linux/test_platform_linux.cpp Outdated Show resolved Hide resolved
src/client/gui/lib/vm_details/cpus_slider.dart Outdated Show resolved Hide resolved
src/client/gui/lib/vm_details/ram_slider.dart Show resolved Hide resolved
@levkropp levkropp force-pushed the daemon-cpu-disk-usage branch 2 times, most recently from a5f591c to db8d38b Compare January 10, 2025 18:53
Copy link
Contributor

@andrei-toterman andrei-toterman left a comment

Choose a reason for hiding this comment

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

Thanks, @levkropp! Good job!

Copy link
Collaborator

@ricab ricab left a comment

Choose a reason for hiding this comment

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

Well done Lev, thanks! Just a couple of small details inline.

src/platform/platform_unix.cpp Show resolved Hide resolved
src/platform/platform_unix.cpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@ricab ricab left a comment

Choose a reason for hiding this comment

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

Looking good @levkropp, thanks!

* edit DaemonInfoReply proto call to have cpus & memory
* implement basic cpu and mem usage getter for linux in daemon.cpp
* implement basic hook for daemonInfoProvider in providers.dart and launch_form.dart
* cpus is now a uint32 in multipass.proto
* removed LinuxSystemInfo struct and replaced with two functions
* get_cpus() and get_total_ram() are now platform functions with a linux implementation
* configuring a VM's resources now uses daemonInfoProvider properly
* cpu, ram, and disk sliders get from daemonInfoProvder in providers.dart instead of their own defined providers
@levkropp levkropp force-pushed the daemon-cpu-disk-usage branch from 64da0bb to 96f5187 Compare January 15, 2025 14:42
@andrei-toterman andrei-toterman added this pull request to the merge queue Jan 15, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 15, 2025
@ricab ricab merged commit cbcb5e7 into main Jan 16, 2025
12 of 14 checks passed
@ricab ricab deleted the daemon-cpu-disk-usage branch January 16, 2025 11:45
@ricab ricab added this to the 1.15.1 milestone Jan 17, 2025
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