GNOME Bugzilla – Bug 632188
Not enough CPU usage line graph colors for high core count systems
Last modified: 2012-01-28 01:31:31 UTC
I use a 24-core Xeon system at work, and system-monitor only colors the first 4 CPU usage lines different colors, then the other 20 lines are all red. Probably a random RGB triple should be generated for each line (with manually-picked aesthetic colors for the first 4 or 8 cores maybe)?
...or a better scheme, spiral inwards around the HSV wheel, fitting all colors into a spiral curve and either keeping the angular increment constant or varying the angle and arriving near the center of the color wheel after a fixed number of turns?
I have a 16-core PC and have a similar problem. All I see is a bunch of red lines.
Created attachment 202177 [details] [review] Add support for more than four cpu graph colors Although I have not changed the way colors are selected for more than 4 cores, I have a small patch attached, which at least allows to store more than 4 colors for the cpu graphs. It's my first time I changed something inside the gnome sources, so please let me know if this is a feasible way of doing things :-)
Review of attachment 202177 [details] [review]: I like this patch, it seems to be clear and should work. The migration case however is not covered, that being the only inconvenience, so people who have already configured their first 4 CPU colors will have to reconfigure them, but I think they should be able to live with that, as migrating would mean to keep both cpu_color1-4 and cpu_colors around, and the code handling the migration.
Thanks for the review, glad to hear the patch looks sane to you. I did not really think about migration when writing the patch, since I had no idea how to remove the cpu_color% properties from the schema once the migration was performed (and having these properties still lurking around was not the ideal solution for me). Or maybe I'm simply not understanding how settings are managed and there is a rather simple solution for this problem. If you can point me to any direction, I'm glad to learn more (even if its not a 'must' have feature) :-)
Created attachment 204835 [details] [review] Change GSettings type for CPU colors to a(us) I've actually implemented this a bit differently. Instead of a simple string I'm using a GVariant of type a(us) where the u is the CPU index and the string is the color. This is type safe, avoids the string manipulation and associated the color with a CPU index. I'm not going to worry about the migration since it is likely to effect a rather small number of users. Seem to work for me. Can I get a review of this?
Review of attachment 204835 [details] [review]: Tested it and works well for me too, and the code also looks fine.
(In reply to comment #7) > Review of attachment 204835 [details] [review]: > > Tested it and works well for me too, and the code also looks fine. Thanks I've pushed this in commit f13ddd3aa0015076961b7764b9c0f7eef79cd3b9. I'm closing this although there is still the issue that one must use a tool such as dconf-editor to change the colors of CPUs 4-n. I'll open another big for that.
Reopening this, as we've overlooked a small fact, which I've found while investigating bug 668317: in callbacks.cpp:cb_cpu_color_changed you only iterate over the number of the children of the cpu-colors GVariant, which will allways be 4, as the default value has 4 elements.
Created attachment 205725 [details] [review] Proposed patch With this patch added, on startup procman rebuilds the gvariant array, and if it has less color than the number of cores, appends the required number of colors to the gvariant list and saves the new list back into gsettings. Without this extension, this bug is not fixed at all, and system monitor crashes on computers with more cores than 4 (because the colors are stored in the gconf default only for 4 cores), see bug #668317.
Thanks. See commit 6e966d0cfca266d5f1ea02995b7debc198ff636d.