GNOME Bugzilla – Bug 787549
Add disk-io monitoring support
Last modified: 2021-05-25 17:31:57 UTC
Last version libgtop have support for monitoring disk-io, so we can implement it in Usage too.
Created attachment 359531 [details] [review] system-monitor: Add support for Disk i/o monitoring Add support for monitoring disk i/o for processes in backend.
Created attachment 359532 [details] [review] disk-subview: Add UI for disk-io monitoring Add subview and graph for disk-io monitoring.
Review of attachment 359532 [details] [review]: ::: src/disk-sub-view.vala @@ +32,3 @@ + label.set_use_markup(true); + label.margin_top = 25; + label.margin_bottom = 15; Couldn't this all be made with gtk+ composite templates?
Created attachment 361809 [details] [review] sub-views: Do not show all widgets in performance subviews To make work default hiding widgets in .ui files (for widgets in performance subviews), we must not call show_all in performance view.
Created attachment 361810 [details] [review] process-list-box: Change private type variable to public property
Created attachment 361811 [details] [review] disk-subview: Add UI for disk-io monitoring Add subview and graph for disk-io monitoring. Updated version with gtk+ composite template.
Created attachment 361813 [details] [review] system-monitor: Add support for Disk i/o monitoring Add support for monitoring disk i/o for processes in backend. I accidentally marked this patch as obslolvete, so I attach it again. This patch is to be applied first, then the rest!
Created attachment 361814 [details] [review] disk-subview: Add UI for disk-io monitoring Add subview and graph for disk-io monitoring. Edited: Added missing row with disk_graph.ensure().
Review of attachment 361810 [details] [review]: this one doesn't apply on top of master.
Review of attachment 361813 [details] [review]: ::: meson.build @@ +7,3 @@ gio = dependency('gio-2.0') gtk = dependency('gtk+-3.0', version : '>=3.20.10') +gtop = dependency('libgtop-2.0', version : '>=2.37.2') A dependency bump could be a commit of its own. ::: src/disk-monitor.vala @@ +32,3 @@ + } + + public uint64 get_read_bytes() use vala's getter and setter. @@ +37,3 @@ + } + + public uint64 get_write_bytes() ditto. @@ +53,3 @@ + process.disk_write = proc_io.disk_wbytes - process.disk_write_last; + process.disk_write_last = proc_io.disk_wbytes; + write_bytes += process.disk_write; Wouldn't be convenient for Process to have a proc_io reference on its own? If not, that's ok. ::: src/system-monitor.vala @@ +87,3 @@ } + public List<unowned Process> get_disk_processes() Could also be a getter in the object's property. @@ +427,3 @@ mem_usage += sub_process.get_mem_usage(); + disk_read += sub_process.disk_read; + disk_write += sub_process.disk_write; indentation?
Review of attachment 361814 [details] [review]: ::: data/ui/disk-sub-view.ui @@ +15,3 @@ + <property name="can_focus">False</property> + <child> + <object class="UsageBetterBox"> What's the gain of having both GtkViewport and UsageBetterBox here instead of going directly to the GtkBox? ::: src/disk-graph-table.vala @@ +1,3 @@ +/* disk-graph-table.vala + * + * Copyright (C) 2017 Red Hat, Inc. 2018 ::: src/disk-sub-view.vala @@ +37,3 @@ + process_list_box.type = ProcessListBoxType.DISK; + + SystemMonitor.get_default().disk_processes_ready.connect(() => If you define a GtkStack where one child contains the process list and the other contains the spinner, we could simplify a lot of things in this class, instead of doing a machine state with each of the widgets "visible" property and the "active" property of the spinner.
Review of attachment 361809 [details] [review]: I hope in later developments we start moving these widget instantiations into the UI files, where we can toggle their visibility in a more organized way.
(In reply to Felipe Borges) Thanks for review. I updated patches according your suggestions. I uploaded new patches as MR on gitlab: https://gitlab.gnome.org/GNOME/gnome-usage/merge_requests/32/ If we can switch to gitlab for this bug, for easiest collaboration. Thanks!
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org. As part of that, we are mass-closing older open tickets in bugzilla.gnome.org which have not seen updates for a longer time (resources are unfortunately quite limited so not every ticket can get handled). If you can still reproduce the situation described in this ticket in a recent and supported software version, then please follow https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines and create a new enhancement request ticket at https://gitlab.gnome.org/GNOME/gnome-usage/-/issues/ Thank you for your understanding and your help.