[Goodies-dev] [Bug 16013] New: increased CPU/memory usage over time [PATCH]

[bugzilla-daemon at xfce.org]

https://bugzilla.xfce.org/show_bug.cgi?id=16013

            Bug ID: 16013
           Summary: increased CPU/memory usage over time [PATCH]
    Classification: Panel Plugins
           Product: Xfce4-cpufreq-plugin
           Version: 1.2.1
          Hardware: PC (x86_64)
                OS: Linux
            Status: NEW
          Severity: normal
          Priority: Medium
         Component: General
          Assignee: goodies-dev at xfce.org
          Reporter: dwksrc at gmail.com
  Target Milestone: ---

I am running xfce4-cpufreq-plugin 1.2.1-1 on Debian. I noticed that it ends up
using increasingly more CPU and RAM over several days, to the point where it
uses 50% CPU and has a 600MB heap (while it started with 4MB). I verified that
it is not performing additional system calls, which was my first guess, and
indeed /proc/X/stat shows that it is spending these CPU cycles in userspace. I
dumped its heap from gdb and found half a million instances of the string
"Sans". I traced the code and found that cpuFreq->layout_changed was being set
to TRUE every update (once per second in my case). The root cause of this is
the following bug:

xfce4-cpufreq-plugin.c:
237         if (label_size.width > cpuFreq->label_max_width)
238           cpuFreq->label_max_width = label_size.width;
239         cpuFreq->layout_changed = TRUE;

It always sets layout_changed to TRUE even when width==label_max_width (the
normal case). This should be

237         if (label_size.width > cpuFreq->label_max_width)
238           cpuFreq->layout_changed = TRUE;
239         cpuFreq->label_max_width = label_size.width;

Luckily this snippet appears correctly in the other place it was copy pasted in
the file. Now it doesn't update repeatedly, which fixes the problem for me. But
there is clearly a memory leak of some sort. After looking very hard at the
code in cpufreq_label_set_font(), I think I found it.

78     provider = gtk_css_provider_new ();
79 
80     gtk_css_provider_load_from_data (provider, css, -1, NULL);
81     gtk_style_context_add_provider (
82       GTK_STYLE_CONTEXT (gtk_widget_get_style_context (GTK_WIDGET
(cpuFreq->label))),
83       GTK_STYLE_PROVIDER (provider),
GTK_STYLE_PROVIDER_PRIORITY_APPLICATION);

This gets called every time a font is loaded. It adds a new GtkStyleProvider
but *doesn't remove the old one*. Hence, over time, many many GtkStyleProviders
exist all tied to the label widget. My suggested fix is to remember the old
pointer so that it can be removed. My complete patch is as follows:

diff --git a/xfce4-cpufreq-plugin.c b/xfce4-cpufreq-plugin.c
index e886121..e2282f3 100644
--- a/xfce4-cpufreq-plugin.c
+++ b/xfce4-cpufreq-plugin.c
@@ -75,7 +75,15 @@ cpufreq_label_set_font (void)

   if (css)
   {
+    if(cpuFreq->label_css_provider)
+    {
+      gtk_style_context_remove_provider (
+        GTK_STYLE_CONTEXT (gtk_widget_get_style_context (GTK_WIDGET
(cpuFreq->label))),
+        GTK_STYLE_PROVIDER (cpuFreq->label_css_provider));
+    }
+
     provider = gtk_css_provider_new ();
+    cpuFreq->label_css_provider = provider;

     gtk_css_provider_load_from_data (provider, css, -1, NULL);
     gtk_style_context_add_provider (
@@ -235,8 +243,8 @@ cpufreq_update_label (CpuInfo *cpu)
       else
       {
         if (label_size.width > cpuFreq->label_max_width)
-          cpuFreq->label_max_width = label_size.width;
-        cpuFreq->layout_changed = TRUE;
+          cpuFreq->layout_changed = TRUE;
+        cpuFreq->label_max_width = label_size.width;
       }
     }
   }
@@ -551,6 +559,7 @@ cpufreq_prepare_label (CpuFreqPlugin *cpufreq)
   {
     gtk_widget_destroy (cpufreq->label);
     cpufreq->label = NULL;
+    cpufreq->label_css_provider = NULL;
   }

   if (cpuFreq->options->show_label_freq ||
cpuFreq->options->show_label_governor)
@@ -576,6 +585,7 @@ cpufreq_widgets (void)
   css = g_strdup_printf("button { padding: 0px; }");

   provider = gtk_css_provider_new ();
+  cpuFreq->label_css_provider = provider;
   gtk_css_provider_load_from_data (provider, css, -1, NULL);
   gtk_style_context_add_provider (
     GTK_STYLE_CONTEXT (gtk_widget_get_style_context (GTK_WIDGET
(cpuFreq->button))),
diff --git a/xfce4-cpufreq-plugin.h b/xfce4-cpufreq-plugin.h
index a6895e4..f163e3c 100644
--- a/xfce4-cpufreq-plugin.h
+++ b/xfce4-cpufreq-plugin.h
@@ -86,6 +86,7 @@ typedef struct

   /* Widgets */
   GtkWidget *button, *box, *icon, *label;
+  GtkCssProvider *label_css_provider;
   gboolean layout_changed;
   gint label_max_width;

I am successfully running this modified version on my system. I will report
back if it continues to use excessive cpu or memory.

P.S. I initially thought there was a bug with mixing up width and height around
line 222, but upon further reflection it is correct. The max_width variable
simply stores a height value in the vertical case.

-- 
You are receiving this mail because:
You are the assignee for the bug.